buglwIP - A Lightweight TCP/IP stack - Bugs: bug #43173, pppos_input() corrupts memory if...

 
 

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

bug #43173: pppos_input() corrupts memory if IP_FORWARD is enabled

Submitted by:  Ivan Delamer <idelamer>
Submitted on:  Mon 08 Sep 2014 05:16:08 PM UTC  
 
Category:  PPP Severity:  4 - Important
Item Group:  Crash Error Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed Planned Release:  None
lwIP version:  git head

( Jump to the original submission)

Sun 12 Oct 2014 04:42:50 PM UTC, comment #6: 

Oh dear, looks like I have been trapped by the custom way the PPPoS stack is dealing with PBUF.

Anyway, I carefully checked and tested the proposed patch, and it works, thank you ;-)

Sylvain Rochet <gradator>
Project Member
Fri 19 Sep 2014 04:34:01 AM UTC, comment #5: 

I'm traveling so I can test this next week, although I don't have a PPP->Ethernet forwarding scenario. I can still test that it doesn't crash.

Ivan Delamer <idelamer>
Project Member
Wed 17 Sep 2014 07:33:10 PM UTC, comment #4: 

Here's a patch of how I would fix it.

Just tried for a very long time to get either one of my windows XP VM or CoLinux Debian to accept (simulated) lwIP's PPP, but failed, so I'm pretty cut off from PPP development right now :-(

(file #32123)

Simon Goldschmidt <goldsimon>
Project Administrator
Tue 16 Sep 2014 08:14:15 PM UTC, comment #3: 

I have a fix where I allocate PBUF_LINK_HLEN as 'len' for the first pbuf (head only) and hide it before passing to pppos_input_callback. However, I cannot test it. If you could test it, I'd just commit it (it compiles, so...)

Simon Goldschmidt <goldsimon>
Project Administrator
Tue 16 Sep 2014 07:53:50 PM UTC, comment #2: 

OK, rings a bell, I had lightly followed that discussion.

So maybe we just need to do that PBUF_LINK offset if pxHead == pxTail, and make sure we only write until PBUF_LEN-PBUF_LINK for that first packet...

Ivan Delamer <idelamer>
Project Member
Tue 16 Sep 2014 06:44:32 PM UTC, comment #1: 
Simon Goldschmidt <goldsimon>
Project Administrator
Mon 08 Sep 2014 05:16:08 PM UTC, original submission:  

This was introduced in commit 4283ecf7748ccf7ab41fc09b7d6d4acb1f7f4444

If IP forward is enabled, a PBUF_POOL with PBUF_LINK offset is allocated.

In my setup, this is a 256-byte buffer with 16-byte offset.

But the code still writes until len == 256, which with 16-byte offset is out of bounds. In pcrx->in_head this is off by 10 bytes (due to helper header inserted) and the rest is off by 16.

It is also unnecessary to allocate PBUF_LINK offset for pbufs other than the first one (pcrx->in_head ).

Won't Etharp or netif->output pre-pend an extra pbuf for link-layer headers?

Ivan Delamer <idelamer>
Project Member

 

Attached Files
file #32123:  possible_fix_43173 added by goldsimon (4KiB - 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 gradator (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by idelamer (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 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-10-12 gradator StatusNone => Fixed
        Open/ClosedOpen => Closed
    2014-09-17 goldsimon Attached File- => Added possible_fix_43173, #32123

    Back to the top


    Powered by Savane 3.5