patchlwIP - A Lightweight TCP/IP stack - Patches: patch #10121, bug: httpd creates circular linked...

 
 

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

patch #10121: bug: httpd creates circular linked list that causes an infinite loop

Submitter:  Peter Lawrence <majbthrd>
Submitted:  Wed 13 Oct 2021 09:52:03 PM UTC
   
 
Category:  Contrib Priority:  6
Status:  Works For Me Privacy:  Public
Assigned to:  goldsimon Open/Closed:  Closed
Planned Release:  None

Jump to the original submission

Fri 04 Oct 2024 12:34:58 PM UTC, comment #7: 

Thanks again! I reported the issue to the guys over at TinyUSB: https://github.com/hathach/tinyusb/issues/2828

> I wonder how that works as you'll end up with double freed pfus.


It did not work, at least not for me. I would get the ref count zero panic so I added some test to only free when ref > 0 and that's how I got that circular buffer panic cause I was stealing ownership of the buffer from httpd.

As far as I'm concerned you can close that ticket.
Btw looks like you guys are way behind on issues triage and patches reviews, looks like you have some backlog from 4 or 5 years maybe. I opened a ticket a few weeks back and mentioned it could now be closed but no action was taken I believe. That's off topic though. I did post recently on the mailing list about apparent lack of activities but that went unanswered too.

At least I'm glad you could help with this one.

Slion <slion>
Fri 04 Oct 2024 08:21:34 AM UTC, comment #6: 

Yes, that's wrong. I wonder how that works as you'll end up with double freed pfus.

We do have examples in our repository:
https://github.com/lwip-tcpip/lwip/blob/master/contrib/examples/ethernetif/ethernetif.c

Pbuf_free must only be called when the input function returns != ERR_OK!

Simon Goldschmidt <goldsimon>
Group administrator
Fri 04 Oct 2024 07:43:35 AM UTC, comment #5: 

Thanks for clarifying that. I think I'm starting to understand what going on.

So when you feed the stack from ethernet_input() you basically surrender ownership of the given pbuf to the stack or apps and should not be calling pbuf_free() when ethernet_input() returns right?

However that's exactly what this example does:
https://github.com/hathach/tinyusb/blob/master/examples/device/net_lwip_webserver/src/main.c

Slion <slion>
Fri 04 Oct 2024 06:47:22 AM UTC, comment #4: 

Re comment #3:
I don't understand the part where the assumption of pbuf ownership should be wrong. That assumption is right and if it can't be right, the driver is wrong and will not work with other apps, too.

If this bug only happens with those drivers, fix the drivers and we can close this bug.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 03 Oct 2024 07:47:26 PM UTC, comment #3: 

Basically LWIP_HTTPD_SUPPORT_REQUESTLIST makes the assumption that it can take ownership of the pbuf which is wrong as pbuf could have been allocated from a pool like in my case and thus we keep reusing them.

Thanks for mentioning the network interface driver that put me on the right track.

Slion <slion>
Thu 03 Oct 2024 07:30:59 PM UTC, comment #2: 

Same issue on Raspberry Pi Pico. It only works if you have a pbuf big enough so that you get your whole request in a single one and pbuf_cat is never called. Apparently setting TCP_MSS to 1500 makes for large enough pbufs but seems to be causing a bunch of other issues where mem_malloc often fails.

Thanks for adding the assert it's rather useful.

Slion <slion>
Thu 12 Oct 2023 07:36:50 PM UTC, comment #1: 

I don't get the same problem you observed when connecting via telnet and pressing enter twice: I get two distinct pbufs. I guess you have a problem somewhere with your netif driver that results in getting the same pbuf twice here.

To detect such problems, I've added an assert to pbuf_cat() that h and t are not the same: although that doesn't every loop, it might detect this bug here.

Good luck with finding your actual bug! ;-)

Simon Goldschmidt <goldsimon>
Group administrator
Wed 13 Oct 2021 09:52:03 PM UTC, original submission:  

This pertains to the httpd service in ./src/apps/http/httpd.c

Unless the user has specifically overridden the normally enabled LWIP_HTTPD_SUPPORT_REQUESTLIST, the right input sequence will cause httpd to go into an infinite loop.

To reproduce, telnet to the httpd service port and press the ENTER key twice.

What I observe is that this test procedure results in two calls to the httpd handler with the same pbuf pointer.  In the code below the comment "enqueue the pbuf", there is a call to pbuf_cat() to concatenate the existing pbuf to itself, and in the later pbuf_len() call will then proceed to count this circular linked list for the rest of time.

Peter Lawrence <majbthrd>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #52096:  httpd.diff added by majbthrd (483B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-10-12 goldsimon StatusNone Works For Me
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2021-10-13 majbthrd Attached File- Added httpd.diff, #52096

    Back to the top

    Powered by Savane 3.15-e6e5.
    Corresponding source code