lwIP - A Lightweight TCP/IP stack - Bugs: bug #27576, pbuf_realloc will assert or crash...
You are not allowed to post comments on this tracker with your current authentication level.
bug #27576: pbuf_realloc will assert or crash on a non-chained pbuf list
Submitter: | Bill Auerbach <billauerbach> | ||
Submitted: | Thu 01 Oct 2009 09:16:11 PM UTC | ||
Category: | pbufs | Severity: | 3 - Normal |
Item Group: | Crash Error | Status: | Fixed |
Privacy: | Public | Assigned to: | goldsimon |
Open/Closed: | Closed | Planned Release: | None |
lwIP version: | CVS Head |
Jump to the original submission
Fri 09 Oct 2009 08:05:53 PM UTC, comment #12: |
Simon Goldschmidt <goldsimon> |
Tue 06 Oct 2009 01:24:55 PM UTC, comment #11: Since you're talking about including this in ethernetif.c, I would think a little more documentation in low_level_input on what is expected of the returned pbuf is sufficient. I didn't need a runtime check as much as I simply needed to know what the requirements are doing my Ethernet driver.
|
Bill Auerbach <billauerbach> |
Tue 06 Oct 2009 10:26:08 AM UTC, comment #10: I was thinking this check would go into ip_input(). Very much debug only. Ideally it would iterate a pbuf chain and check all links in the chain. |
Kieran Mansley <kieranm> |
Mon 05 Oct 2009 08:12:36 PM UTC, comment #9: If it's done in pbuf_free, the check will be less often and will also catch pbuf corruption if this mismatch should occur after pbuf_realloc.
|
Bill Auerbach <billauerbach> |
Mon 05 Oct 2009 08:01:39 PM UTC, comment #8: The question is where would you place such an assert? You would have to place it every function that uses p->len or p->tot_len.
|
Simon Goldschmidt <goldsimon> |
Fri 02 Oct 2009 02:31:58 PM UTC, comment #7: A debug assert would be appropriate because lwIP will run fine even though tot_len != len which is an incorrect state for a pbuf. |
Bill Auerbach <billauerbach> |
Fri 02 Oct 2009 02:00:02 PM UTC, comment #6: You're right!!! I update len with the incoming packet, but not tot_len.
|
Bill Auerbach <billauerbach> |
Fri 02 Oct 2009 01:47:06 PM UTC, comment #5: I see you mentioned that this is a single pbuf, not a chain. In that case it should be even simpler: check that p->tot_len equals p->len for that pbuf. My guess is that you'll see a discrepancy, and this is why it goes wrong. |
Kieran Mansley <kieranm> |
Fri 02 Oct 2009 01:45:49 PM UTC, comment #4: I've had a look at the code and we do have a check for ip length:
|
Kieran Mansley <kieranm> |
Fri 02 Oct 2009 12:30:04 PM UTC, comment #3: Funny you should say that - that's exactly the bug I'm chasing on my end - every once in a while the packet is short by 1 to 31 bytes. |
Bill Auerbach <billauerbach> |
Fri 02 Oct 2009 07:46:15 AM UTC, comment #2: Sounds like it is bad because the frame is truncated (i.e. less data received than the IP header claims should be there). We should have a check for this. |
Kieran Mansley <kieranm> |
Thu 01 Oct 2009 09:45:03 PM UTC, comment #1: I see pbuf_realloc is after the header length and checksum are checked, but I am still getting the assert intermittently as I deal with incoming packets that I know are bad.
|
Bill Auerbach <billauerbach> |
Thu 01 Oct 2009 09:16:11 PM UTC, original submission:
pbuf_realloc can be called from ip_input with a bad packet before the packet is known to be bad because the IP header is bad. This causes an assertion or crash (when LWIP_NOASSERT defined).
|
Bill Auerbach <billauerbach> |
No files currently attached
Depends on the following items: None found
Items that depend on this one: None found
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 3 latest changes.
Date | Changed by | Updated Field | Previous Value | => | Replaced by |
---|---|---|---|---|---|
2009-10-09 | goldsimon | Status | None | Fixed | |
Assigned to | None | goldsimon | |||
Open/Closed | Open | Closed |
Powered by Savane 3.13-02a9.
Corresponding source code
Added a comment in low_level_input as requested.