buglwIP - A Lightweight TCP/IP stack - Bugs: bug #15659, IP fragmentation and reassembly...

 
 

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

bug #15659: IP fragmentation and reassembly crashes

Submitter:  Christiaan Simons <christiaans>
Submitted:  Wed 08 Feb 2006 11:53:28 AM UTC
   
 
Category:  IPv4 Severity:  2 - Minor
Item Group:  Crash Error Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed Planned Release:  None
lwIP version:  None

Jump to the original submission

Wed 16 May 2007 10:46:01 AM UTC, comment #8: 

I've added ASSERTs in CVS to make sure this bug is fixed.

Closing this as fixed.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 19 Apr 2007 11:42:39 AM UTC, comment #7: 

Dmitry, can you explain why Jons correction only works for 32-bit pointers?

Aside from that, I think this bug is already fixed by pbuf.c cvs version 1.86 (see patch #5796) and this bug can be closed.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 26 Feb 2007 07:35:26 PM UTC, comment #6: 

Jon Chiang's correction works only for 32-bit pointers.
So, I want to propose an alternative: to adjust 'offset' instead of 'payload'. This solution is more simple and should work for any platform. I have attached a patch that do that.

(file #12049)

Dmitry Potapov <dpotapov>
Mon 26 Feb 2007 01:07:39 PM UTC, comment #5: 

It's not clear from comment #4 exactly what change is required.  Could you illustrate it with a patch or diff?

Kieran Mansley <kieranm>
Group Member
Mon 13 Nov 2006 02:49:15 AM UTC, comment #4: 

The root caused is MEM_ALIGN using incorrectly in pbu_alloc for POOL case.

When the offset is PBUF_LINK:14 that out of MEM_ALIGN:4, the pbuf->len would be assigned a value which cause the p->payload + p->len out of the current pbuf size, and return to ip_frag()
the memcpy() overwrite the pbuf pool then the data abort exception occurs!

The statement below is my solution to fix this bug.


/* make the payload pointer point 'offset' bytes into pbuf data memory */
    p->payload = MEM_ALIGN((void )((u8_t )p + (sizeof(struct pbuf) + offset)));
    LWIP_ASSERT("pbuf_alloc: pbuf p->payload properly aligned",
            ((mem_ptr_t)p->payload % MEM_ALIGNMENT) == 0);
    /* the total length of the pbuf chain is the requested size */
    p->tot_len = length;
    /* set the length of the first pbuf in the chain */
    pbuf_end = (u32)p + sizeof(struct pbuf) + PBUF_POOL_BUFSIZE;
    p->len = length > pbuf_end - (u32)p->payload ? pbuf_end - (u32)p->payload :
    length;

Jon Chiang <xioujon>
Fri 03 Mar 2006 12:37:09 PM UTC, comment #3: 

Fixed missing pbuf_alloc() return checks. Tests on a real world network (instead of the tapif) show poor performace. A regular ping -s 4000 (at 1 sec interval) on a LAN gives 23% packet loss.

Christiaan Simons <christiaans>
Group Member
Wed 01 Mar 2006 03:53:32 PM UTC, comment #2: 

I'm trying to use a sys_timeout handler for calling ip_reass_tmr(), but this doesn't seem to run reliably. If this timer doesn't run, reassembly will certainly fail after a while.

Christiaan Simons <christiaans>
Group Member
Fri 10 Feb 2006 08:22:56 AM UTC, comment #1: 

The actual bugs were in contrib/ports/unix/proj/minimal/mintapif. The robustness of the reassembly code should be reviewed though, I don't like segfaults...

Christiaan Simons <christiaans>
Group Member
Wed 08 Feb 2006 11:53:28 AM UTC, original submission:  

I've found the IP fragmentation and reassembly code crashes when using the raw-API minimal example. It looks like the incoming pbuf chain isn't read properly. Also outgoing fragments seem to be too large for the tunif. I'm working on a solution for this.

Christiaan Simons <christiaans>
Group Member

 

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

Attached Files
file #12049:  pbuf.c.patch added by dpotapov (733B - text/x-patch - Fix for pbuf_alloc)

 

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 dpotapov (Updated the item)
  • -email is unavailable- added by kieranm (Posted a comment)
  •  

    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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-05-16 goldsimon StatusPostponed Fixed
        Open/ClosedOpen Closed
    2007-02-26 dpotapov Attached File- Added pbuf.c.patch, #12049
    2006-05-02 christiaans StatusIn Progress Postponed
        Assigned tochristiaans None
    2006-02-10 christiaans Severity4 - Important 2 - Minor
    2006-02-08 christiaans StatusNone In Progress

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code