[lwip-devel] [bug #56077] len member wrong in pool allocated pbuf

[Karl Heinz Buchegger]

URL:
  <https://savannah.nongnu.org/bugs/?56077>

                 Summary: len member wrong in pool allocated pbuf
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: kbuchegg
            Submitted on: Fri 05 Apr 2019 11:52:38 AM UTC
                Category: pbufs
                Severity: 3 - Normal
              Item Group: None
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None
            lwIP version: git head

    _______________________________________________________

Details:

Can someone please confirm if this is really a bug or am I missing something

According to my understanding of the meaning of the len (and total_len) member
in struct pbuf, they both describe the actually usable length of the payload
memory (or the total of it)
>From studying the source code all over lwip, this is my impression of the
meaning. Note that this is the only interpretation which makes sense from the
point of view of the user of a pool allocated pbuf chain.

if this is true, then there is a bug in the case of a PBUF_POOL allocated
buffer.

Instead of
    p->len = LWIP_MIN(length, PBUF_POOL_BUFSIZE_ALIGNED -
LWIP_MEM_ALIGN_SIZE(offset));
    
it has to read
    p->len = LWIP_MIN(length, PBUF_POOL_BUFSIZE_ALIGNED -
LWIP_MEM_ALIGN_SIZE(SIZEOF_STRUCT_PBUF + offset));


and further down, instead of
    q->len = LWIP_MIN((u16_t)rem_len, PBUF_POOL_BUFSIZE_ALIGNED);
    
it has to read
      q->len = LWIP_MIN((u16_t)rem_len, PBUF_POOL_BUFSIZE_ALIGNED -
SIZEOF_STRUCT_PBUF);
      
(that is in both cases: subtract the size of the pbuf struct from the
available payload length also, since the pbuf structure is allocated inside
the memory obtained from the pool allocator, and thus this memory is not
usable for storing payloads)

Also note, that there is a bug in the immediatly following assertions.
The first one reads
LWIP_ASSERT("check p->payload + p->len does not overflow pbuf",
                ((u8_t*)p->payload + p->len <=
                 (u8_t*)p + SIZEOF_STRUCT_PBUF + PBUF_POOL_BUFSIZE_ALIGNED));

which seems to take care of the SIZEOF_STRUCT_PBUF as beeing not part of the
usable payload length, but does not include the offset.

while the second one (inside the loop allocating the remaining required
length) reads
LWIP_ASSERT("check p->payload + p->len does not overflow pbuf",
                  ((u8_t*)p->payload + p->len <=
                   (u8_t*)p + SIZEOF_STRUCT_PBUF +
PBUF_POOL_BUFSIZE_ALIGNED));

which is totaly flawed, since the thing the assertion is all about is pointed
to by q and not by p (but interestingly supports the understanding that
SIZEOF_STRUCT_PBUF is not included in the value of len)

Once the len member is corrected, total_len will be correct automatically and
more important, pbuf_alloc will alloacte a chain of pool allocated pbuf's with
the correct size for storing the requested amount of bytes as payload.






    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/bugs/?56077>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/