patchlwIP - A Lightweight TCP/IP stack - Patches: patch #9430, altcp API multiples fixes


You are not allowed to post comments on this tracker with your current authentication level.

patch #9430: altcp API multiples fixes

Submitted by:  David GIRAULT <dgirault>
Submitted on:  Mon 07 Aug 2017 02:18:41 PM UTC  
Category:  apps Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  Simon Goldschmidt <goldsimon> Open/Closed:  Closed
Planned Release:  2.1.0

( Jump to the original submission)

Tue 16 Jan 2018 08:49:42 PM UTC, comment #8:

Sorry, could you pass me to the function where I removed that?

In general, in (al-)TCP a layer should free its memory when it is closed. In the simple case of "app <-> tls <-> tcp", the inner_conn is tcp, which should free itself when both communication directions are closed.

Do you see memory leaks in this area?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 16 Jan 2018 08:28:40 AM UTC, comment #7:

Currently rebasing my own master to yours, I understood that you revert the altcp_free(inner_conn), because of double free, but I don't see where the inner_conn is freed... Could you give me some tips?


David GIRAULT <dgirault>
Wed 15 Nov 2017 09:00:04 PM UTC, comment #6:

After finally finding the time to continue on this: my apologies, the mbedtls part was nearly correct.

I found a double-free though, which I fixed in commit bd2e82082955aa3fd64cddda98d69acf5e918032.

Thanks for the patch.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 20 Sep 2017 07:19:59 PM UTC, comment #5:

OK, I've pushed the fix to altcp_tcp.c (slightly modified).

However, I'm not sure the mbedtls close/dealloc fixes are correct. It looks like the changes work for your specific handshake abort issue, but there might still be memory leaks in other places? Give me some time to check that further...

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 28 Aug 2017 08:01:36 PM UTC, comment #4:

Thanks. I'll try to review this after fixing bug #51687

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 09 Aug 2017 05:31:46 PM UTC, comment #3:

I'm not really conviced too for the altcp_tcp_close() changes. But, this fix is the only way I got altcp_pcb leaks fixed.
This change also made the altcp layer the same way the tcp layer.
pcb should not be used anymore after closed.

I have additionnal patch to do the same for altcp_mbedtls_close(), in addition of closing inner_conn.

I've just rebased my branch to last master to date (ba7a3204) and attach new patchset with remaining fixes.


(file #41464, file #41465)

David GIRAULT <dgirault>
Tue 08 Aug 2017 10:56:26 AM UTC, comment #2:

I've pushed your patches with 2 changes:
a) I'm not yet convinced by the tcp_close() change: if it returns != ERR_OK, the callbacks are gone, so the pcb might be just leaked
b) instead of exposing altcp_tcp_setup(), I've added altcp_tcp_wrap(), which allocates the altcp_pcb.

Leaving this open for the tcp_close() issue

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 08 Aug 2017 08:56:11 AM UTC, comment #1:

Thanks for the patches! The work on altcp is not finished yet, so any improvement is welcome!

Especially I'm not really happy with the way servers and connections are instantiated. There will be some API change to that, hopefully before 2.1.0...

In the meantime, I'll review your patches :-)

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 07 Aug 2017 02:18:41 PM UTC, original submission:

In our project, we use the altcp layer to support both MQTT and HTTP/HTTPS.

We faced a lot of problems and asserts so I try to fixe them with the following patches.

Thoses patches will resolve the following problems:

- altcp_free() result in memory leak,
- altcp_pcb not freed on lower error callback calls,
- no way to free mbedtls config,
- no way to retreive mbedtls ssl context to check certificates,
- no way to attach existing tcp_pcb to a new altcp_pcb.
- removed an assert in altcp_mbedtls since this condition (rx_pbuf == 0 at end of handshake) can fail.

David GIRAULT <dgirault>



Depends on the following items: None found

Items that depend on this one: None found


Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by dgirault (Submitted the item)

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.




    Follow 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-11-15 goldsimon StatusIn Progress => Done
        Open/ClosedOpen => Closed
        Planned ReleaseNone => 2.1.0
    2017-08-09 dgirault Attached File- => Added 0001-altcp_tcp-free-altcp_pcb-struct-in-altcp_tcp_close.patch, #41464
        Attached File- => Added 0002-altcp_mbedtls-close-and-error-related-fixes.patch, #41465
    2017-08-08 goldsimon StatusNone => In Progress
        Assigned toNone => goldsimon
    2017-08-07 dgirault Attached File- => Added 0005-altcp_tls_mbedtls-remove-rx-pbufs-left-at-end-of-han.patch, #41447
    2017-08-07 dgirault Attached File- => Added 0001-altcp_tls_mbedtls-fix-log-messages-by-include-a-n.patch, #41443
        Attached File- => Added 0002-altcp-fix-altcp_free.patch, #41444
        Attached File- => Added 0003-altcp_tcp-some-fixes.patch, #41445
        Attached File- => Added 0004-altcp_tls-some-fixes.patch, #41446

    Back to the top

    Powered by Savane 3.3