[lwip-devel] [bug #62857] sendmsg(), if called with last item in msg_iov array to have .iov_len=0, build pbuf chain which cannot be cloned

[Alex Riesen]

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

                 Summary: sendmsg(), if called with last item in msg_iov array
to have .iov_len=0, build pbuf chain which cannot be cloned
                 Project: lwIP - A Lightweight TCP/IP stack
               Submitter: raalkml
               Submitted: Wed 03 Aug 2022 11:03:40 AM UTC
                Category: sockets/netconn
                Severity: 3 - Normal
              Item Group: Faulty Behaviour
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None
            lwIP version: 2.1.0


    _______________________________________________________

Follow-up Comments:


-------------------------------------------------------
Date: Wed 03 Aug 2022 11:03:40 AM UTC By: Alex Riesen <raalkml>
Hi,

I call lwip_sendmsg from LwIP 2.1.0 (release) with an UDP socket like this:


  static int udp_tx(int udp_skt, const void *PAYLOAD, size_t PAYLOAD_LENGTH)
  {
    struct iovec iov[2];
    struct msghdr msg;
    msg.msg_name = &remote_addr; /* global */
    msg.msg_namelen = sizeof(remote_addr);
    msg.msg_iov = iov;
    msg.msg_control = NULL;
    msg.msg_controllen = 0;
    msg.msg_flags = 0;
    iov[0].iov_base = &PROTOCOL_MAGIC;
    iov[0].iov_len = sizeof(PROTOCOL_MAGIC); /* 4 bytes */
    iov[1].iov_base = (void *)PAYLOAD;
    iov[1].iov_len = PAYLOAD_LENGTH;
    msg.msg_iovlen = 2;
    return lwip_sendmsg(udp_skt, &msg, 0);
  }


Usually, the payload is a string of bytes, but sometimes the function
"udp_tx" is called with zero-sized payload (end-of-transmission marker).

In those cases, the UDP packet on wire seems to contain random junk
instead of just whatever should be in the PROTOCOL_MAGIC.

After some debugging, it came to the low-level ethernet driver in my project,
which contains code like this:


  /* Check payload alignment in `p` */
  if ((uintptr_t)p->payload % MEM_ALIGNMENT != 0) {
    /* Create a single pbuf long enough to store the whole chain */
    struct pbuf *q = pbuf_clone(PBUF_RAW, PBUF_RAM, p);
    if (!q) ... error out ...
    p = q;
  } ...
  ... Pass p->payload to hardware ...


The Ethernet hardware apparently needs a contiguous, aligned data to
transfer,
and the code tried to provide that.

The problem is that pbuf_clone returns not the expected pbuf containing the
flattened chain given in the "p" argument. The implementation in 2.1.0 (and
if
I'm not mistaken, also in master) will return the allocated, partially filled
buffer, ignoring returned error from pbuf_copy!

The pbuf_copy in this case returns the errors because of this code:


    if ((p_from != NULL) && (p_from->len == p_from->tot_len)) {
      /* don't copy more than one packet! */
      LWIP_ERROR("pbuf_copy() does not allow packet queues!",
                 (p_from->next == NULL), return ERR_VAL;);
    }


On its final iterations, having processed Ethernet, IP, UDP, and the magic
from application, it reaches the zero-sized msg_iov item and returns with an
error code, which pbuf_clone mostly ignores (might print a debug message) and
returns new pbuf to the caller, which has no way to know that it is not the
clone it wanted.

Now, arguably "sendmsg" shouldn't have created such an evil chain of pbufs.
On the other hand, pbuf_clone is a public function and is even prepared to
fail (it returns NULL). On yet another hand, pbuf_copy may simply skip over
such buffers.

Right now, pbuf_clone just silently produces partial clones. And sendmsg
silently
(under some circumstances, admitted) sends corrupted packets.









    _______________________________________________________

Reply to this item at:

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

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