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

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Wilfred
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/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

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

Wilfred
Follow-up Comment #1, bug #56077 (project lwip):

Also I would like to suggest a clearification in the documentation on the
meaning of the len (and total_len) member, since
/** length of this buffer */
is a little bit to vague and in case of PBUF_POOL is not identical to "length
of payload"

    _______________________________________________________

Reply to this item at:

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

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


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

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

Wilfred
Follow-up Comment #2, bug #56077 (project lwip):

I am sorry, I made a mistake.
I was looking at version 2.0.3 and not at the head version.

I just checked the head version. Although the code is different, it suffers
from the same problem: In case of PBUF_POOL the len field has a wrong value
and is to large for the amount of memory available.

    _______________________________________________________

Reply to this item at:

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

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


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

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

Wilfred
Update of bug #56077 (project lwip):

                  Status:                    None => Need Info              

    _______________________________________________________

Follow-up Comment #3:

I don't really get what you're trying to say.

Can you provide a piece of code to reproduce what you think the problem is?
Ideally on the win32 or linux port.

    _______________________________________________________

Reply to this item at:

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

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


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel