buglwIP - A Lightweight TCP/IP stack - Bugs: bug #58441, Invalid PPP data accumulates...

 
 

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

bug #58441: Invalid PPP data accumulates forever

Submitter:  Ashley Duncan <ashesman>
Submitted:  Mon 25 May 2020 03:50:41 AM UTC
   
 
Category:  PPP Severity:  3 - Normal
Item Group:  Change Request Status:  Confirmed
Privacy:  Public Assigned to:  gradator
Open/Closed:  Closed Planned Release:  None
lwIP version:  2.1.1

Thu 22 Oct 2020 10:42:49 PM UTC, comment #4: 

I agree that should be a good candidate, it is not that bad in the wild but the fix is harmless. The branch does not seem to be updated that often, or at least, not day to day, I have no clue if Simon keep a list somewhere or if he just cherry-pick what seem to be fixes when he want to do a release.

Sylvain Rochet <gradator>
Group Member
Tue 20 Oct 2020 04:00:07 AM UTC, comment #3: 

This issue was reported against v2.1.1, so I'm wondering if it should be applied to STABLE-2_1_x branch as well.

Axel Lin <axellin>
Group Member
Tue 20 Oct 2020 03:47:57 AM UTC, comment #2: 

Fixed in commit a39ce0f53b2a57c4e7fbb252efb579a629201868.

Thank you for the report !

Sylvain Rochet <gradator>
Group Member
Tue 26 May 2020 11:49:49 AM UTC, comment #1: 

Indeed, that can be troublesome for systems with multiple netifs as we do not have separate Rx pools for interfaces, starving the Rx pool for garbage is not a really nice behavior. For single netif system that should not be an annoying problem since the worse that can happen is a failure to allocate the single PBUF needed for PPPoS Tx when PPPoS Rx is currently filling the last Rx PBUF, hoping it does not happen at a noticeable rate.

Looks like we need to add a PPPoS MRU (Maximum Receive Unit) size in build time options to prevent that. Validating early the frame header could prevent queuing up to the MRU size but by itself is not enough because a very valid and very long PPP frame can still starve the Rx pool.

Sylvain

Sylvain Rochet <gradator>
Group Member
Mon 25 May 2020 03:50:41 AM UTC, original submission:  

Today I had an issue with the wifi module I use where it crashes and resets, dropping its baud rate in the process and dumping a whole lot of rubbish on the serial port.  This is received as a long series of 0x80, 0x80, 0x80, x80, ... due to the baud rate mismatch.

I did a bit of debugging and noticed that this sequence of data just keeps getting added to a pbuf forever by pppos_input, there are no checks that result in the data being discarded.

Forgive me if this is just a side effect of the PPP protocol that I don't understand that is unavoidable.  I am also unable to propose a suitable solution due to my lack of PPP protocol knowledge.  The correct solution is to discard the UART received bytes that were corrupt as UART would have received them with an error flag.

There is a check at line 629 of pppos.c that is currently disabled.  This check would have caught this situation.

        case PDCONTROL:                 /* Process control field. */
          /* If we don't get a valid control code, restart. */
          if (cur_char == PPP_UI) {
            pppos->in_state = PDPROTOCOL1;
            break;
          }
          /* no break */

#if 0
          else {
            PPPDEBUG(LOG_WARNING,
                     ("pppos_input[%d]: Invalid control <%d>\n", ppp->netif->num, cur_char));
            pppos->in_state = PDSTART;
          }
#endif
          /* Fall through */

Below is some example PPP test data that shows what I received:

~!E<0x00><0x00>7<0x00>\n<0x00><0x00><0xff><0x11>ax<0xac><0x1e><0x00><0xfb><0xac><0x1e><0x00><0xfc><0x00><0x17><0x00><0x17><0x00>#<0xe3><0x82>\r\n+UUWLE:0,6038E0864CC0,5\r\n<0xa6><0xe3>~

<0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80><0x80>

Ashley Duncan <ashesman>

 

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

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by axellin (Posted a comment)
  • -email is unavailable- added by gradator (Posted a comment)
  • -email is unavailable- added by ashesman (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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-10-20 gradator Severity4 - Important 3 - Normal
        Open/ClosedOpen Closed
    2020-05-26 gradator Severity3 - Normal 4 - Important
        Item GroupFaulty Behaviour Change Request
        StatusNone Confirmed
        Assigned toNone gradator

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code