buglwIP - A Lightweight TCP/IP stack - Bugs: bug #35756, header length calculation problem...

 
 

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

bug #35756: header length calculation problem in ppp/vj.c

Submitter:  Arne <alpharne>
Submitted:  Thu 08 Mar 2012 05:31:36 AM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  CVS Head

Tue 27 Mar 2012 06:44:38 PM UTC, comment #3: 

Fixed in both 1.4.1 and master branches.

Thanks for reporting.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 27 Mar 2012 06:39:59 PM UTC, comment #2: 


> If that doesn't work, there's either something wrong with our TCPH_* defines


As it turns out, there is! In my view, the TCPH_OFFSET() macro is invalid: it operates on the first 8 bit of '_hdrlen_rsvd_flags' but that's no valid field.

As a result of this, I chose the wrong macro (TCPH_OFFSET instead of TCPH_HDRLEN when fixing bug #28195 for 1.4.0 (1.3.2 should work correctly) - because of the macro name that matched the previous getth_off().

I'll fix as suggested and remove the TCPH_OFFSET macros (unused).

Simon Goldschmidt <goldsimon>
Group administrator
Thu 08 Mar 2012 06:29:39 AM UTC, comment #1: 

Unfortunately, we're not the experts on that ppp code: it's more or less a direct copy of ppd (https://ppp.samba.org/) with adaptions to lwIP. One of these adaptions is to replace 'getth_off()' with 'TCPH_OFFSET()', and both should return the same TCP header's structure.

If that doesn't work, there's either something wrong with our TCPH_* defines working on '_hdrlen_rsvd_flags' or with your port, but I don't think changing vj.c is the necessary fix?

Simon Goldschmidt <goldsimon>
Group administrator
Thu 08 Mar 2012 05:31:36 AM UTC, original submission:  

The TCPH_OFFSET calls in this file should be replaced by TCPH_HDRLEN.

Right now it uses the reserved bits and the length shifted left by 4 as header length resulting in a possible overwriting of the internal structures as the length can get beyond the maximum valid header length.

Arne <alpharne>

 

(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 goldsimon (Posted a comment)
  • -email is unavailable- added by alpharne (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2012-03-27 goldsimon StatusIn Progress Fixed
        Open/ClosedOpen Closed
        Planned Release 1.4.1
    2012-03-27 goldsimon StatusNone In Progress
        Assigned toNone goldsimon

    Back to the top

    Powered by Savane 3.13-caa5.
    Corresponding source code