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: appsPriority: 5 - Normal
Status: DonePrivacy: Public
Assigned to: Simon Goldschmidt <goldsimon>Open/Closed: Closed
Planned Release: 2.1.0

(Jump to the original submission Jump to the original submission)

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.

David

(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
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by dgirault (Submitted the item)
  •  

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

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 12 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 15 Nov 2017 09:00:04 PM UTCgoldsimonStatusIn Progress=>Done
      Open/ClosedOpen=>Closed
      Planned ReleaseNone=>2.1.0
    Wed 09 Aug 2017 05:31:46 PM UTCdgiraultAttached 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
    Tue 08 Aug 2017 10:56:26 AM UTCgoldsimonStatusNone=>In Progress
      Assigned toNone=>goldsimon
    Mon 07 Aug 2017 02:18:54 PM UTCdgiraultAttached File-=>Added 0005-altcp_tls_mbedtls-remove-rx-pbufs-left-at-end-of-han.patch, #41447
    Mon 07 Aug 2017 02:18:41 PM UTCdgiraultAttached 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.1-cleanup1