[bug #56437] Stack/heap corruption when sending UDP through socket interface

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

[bug #56437] Stack/heap corruption when sending UDP through socket interface

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

                 Summary: Stack/heap corruption when sending UDP through
socket interface
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: gideonz
            Submitted on: Tue 04 Jun 2019 06:35:26 PM UTC
                Category: UDP
                Severity: 3 - Normal
              Item Group: Crash Error
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None
            lwIP version: 1.4.1

    _______________________________________________________

Details:

When using lwIP through its socket interface, stack or heap corruption occurs
when the parameter LWIP_NETIF_TX_SINGLE_PBUF is set to 0, and the user
application tries to send a UDP packet.

The root cause is, that when LWIP_NETIF_TX_SINGLE_PBUF is 0, the stack decides
to use a pbuf in reference mode, by calling __netbuf_ref__, in the function
__lwip_sendto__. The payload pointer points to user data, possibly on the
stack of the caller.

Later, within the tcpip_thread, the header is appended in front of this user
data, because of the following code snippet in :

```  } else if (type == PBUF_REF || type == PBUF_ROM) {
          // just ALWAYS do this, since references are only used to read!
      p->payload = (u8_t *)p->payload - header_size_increment;
```

Nice comment.. Yes, the data is only read, but the area near it is written..
;-)

I think there are actually TWO problems:

1) A pbuf reference should never point to the stack of a caller, because the
actual sending of a packet happens in another thread. It would 'work' in this
particular case, because the caller is stalled using the semaphore until the
packet is sent. But if the driver queues the packets, OR when someone
'improves' the lwIP code by adding a queue mechanism, this will definitely
break.

2) A PBUF_REF should never be chosen if a header needs to be pre-pended. It
would be fine if the core would actually chain the PBUFs. But in case of a
PBUF_REF it explicitly chooses not to.

This problem seems to be addressed in 2.1.2, but I checked: the UDP send still
uses a PBUF_REF for the UDP send function. I believe, from analyzing the code,
that it allocates a new pbuf and chains it, which would be OK. (Despite the
fact that the code to do this should not be in udp.c; it doesn't belong
there.)





    _______________________________________________________

Reply to this item at:

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

_______________________________________________
  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 #56437] Stack/heap corruption when sending UDP through socket interface

Wilfred
Update of bug #56437 (project lwip):

                  Status:                    None => Invalid                
             Open/Closed:                    Open => Closed                

    _______________________________________________________

Follow-up Comment #1:

I think there's only *one* problem: you're using a modified stack. I can't
find that comment nor the code in our released sources.

Nit: that comment even uses C++ style. We're only using C-style comments ;-)

    _______________________________________________________

Reply to this item at:

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

_______________________________________________
  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 #56437] Stack/heap corruption when sending UDP through socket interface

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

Thank you for your reply. I apologize. You are absolutely right about me using
a modified stack. It has been 5 years since I made this [horrible] change. I
verified most code pieces with the repository, but I missed this part;
probably because I was too happy that I finally found the root cause of my
crash.

// The comments are C90 / C99 style.

    _______________________________________________________

Reply to this item at:

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

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


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