patchlwIP - A Lightweight TCP/IP stack - Patches: patch #10132, fix pbuf->len calculation error...

 
 

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

patch #10132: fix pbuf->len calculation error in type PBUF_POOL

Submitted by:  Eric.Wu <duckpowermb>
Submitted on:  Wed 03 Nov 2021 08:42:54 AM UTC
 
Category:  pbufs Priority:  5 - Normal
Status:  Invalid Privacy:  Public
Assigned to:  goldsimon Open/Closed:  Closed
Planned Release:  None
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       No canned response available

 

Thu 30 Dec 2021 10:24:10 PM UTC, comment #3: 

As Erik has written, I do think this is correct. Please reopen if you think otherwise.

Simon Goldschmidt <goldsimon>
Project Administrator
Wed 24 Nov 2021 11:02:04 PM UTC, comment #2: 

The PBUF_POOL mempool is allocated with a special macro that adds the size of the pbuf struct to the requested size:

#define LWIP_PBUF_MEMPOOL(name, num, payload, desc) LWIP_MEMPOOL(name, num, (LWIP_MEM_ALIGN_SIZE(sizeof(struct pbuf)) + LWIP_MEM_ALIGN_SIZE(payload)), desc)

`PBUF_POOL_BUFSIZE_ALIGNED` is how many bytes are available after the struct - so the size of the struct should not need to be subtracted here.

Wouldn't memory be corrupted very quickly if this actually was broken - since this is used for every RX?

Can you add a unit test for this to show the issue (that fails without this patch and works with it)?

Erik Ekman <yarrick>
Project Member
Wed 03 Nov 2021 09:24:34 AM UTC, comment #1: 

I reviewed the code and it indeed seems incorrect.

Maybe it makes more sense to add the "- SIZEOF_STRUCT_PBUF" to the PBUF_POOL_BUFSIZE_ALIGNED define instead, as this is the only user? Then the assert two lines lower is automatically updated too.

Indan Zupancic <indan>
Wed 03 Nov 2021 08:42:54 AM UTC, original submission:  

pbuf.c line:253

qlen = LWIP_MIN(rem_len, (u16_t)(PBUF_POOL_BUFSIZE_ALIGNED - LWIP_MEM_ALIGN_SIZE(offset)));

while

pbuf->playload = LWIP_MEM_ALIGN((u8_t *)q + SIZEOF_STRUCT_PBUF + offset))

there is distance of SIZEOF_STRUCT_PBUF

which may makes a pbuf being overwritten by size of SIZEOF_STRUCT_PBUF

Eric.Wu <duckpowermb>

 

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

Attach Files:
   
   
Comment:
   

Attached Files

 

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 yarrick (Posted a comment)
  • -email is unavailable- added by indan (Posted a comment)
  • -email is unavailable- added by duckpowermb (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.

    Only logged-in users can vote.

     

     

     

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-12-30 goldsimon StatusNone => Invalid
        Assigned toNone => goldsimon
        Open/ClosedOpen => Closed
    2021-11-03 duckpowermb Attached File- => Added pbuf_len_calculation_error.diff, #52192

    Back to the top


    Powered by Savane 3.9