buglwIP - A Lightweight TCP/IP stack - Bugs: bug #25051, lwip_recvfrom problem with udp:...

 
 

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

bug #25051: lwip_recvfrom problem with udp: fromaddr and port uses deleted netbuf

Submitter:  Tamas Somogyi <tsomogyi>
Submitted:  Tue 09 Dec 2008 02:13:47 PM UTC
   
 
Category:  sockets/netconn Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  fbernon
Open/Closed:  Closed Planned Release:  None
lwIP version:  CVS Head

Jump to the original submission

Wed 10 Dec 2008 09:38:22 PM UTC, comment #6: 

Ok, it's check in.

Frédéric Bernon <fbernon>
Group Member
Wed 10 Dec 2008 11:32:14 AM UTC, comment #5: 

Go for it.  Thanks both for finding and fixing this bug.

Kieran Mansley <kieranm>
Group Member
Wed 10 Dec 2008 11:01:37 AM UTC, comment #4: 

Is there any objection to check in the patch ?

Frédéric Bernon <fbernon>
Group Member
Wed 10 Dec 2008 10:58:57 AM UTC, comment #3: 

My device using #17011 has been working fine in the test harness for 3 hours, so the fix is good for me.

Tamas Somogyi <tsomogyi>
Tue 09 Dec 2008 10:58:07 PM UTC, comment #2: 

I attach you a modified lwip_recvfrom. Tell me if it's good for you. In this case, I will check in the change.



(file #17011)

Frédéric Bernon <fbernon>
Group Member
Tue 09 Dec 2008 08:06:19 PM UTC, comment #1: 

It seems you catch a real problem. I will verify that and check in a fix based on the solution.

Frédéric Bernon <fbernon>
Group Member
Tue 09 Dec 2008 02:13:47 PM UTC, original submission:  

At the end of lwip_recvfrom, input address/port is extracted by "addr = netbuf_fromaddr(buf)" for UDP sockets. But "buf" can already be disposed by "netbuf_delete(buf);" in the do-while loop.

So if other threads (re-)use that buffer before the address has been copied to "sin", then it will contain inapropriate data. This is often the case in my application: I'm getting invalid address/port data 3-6 times/hour.

I fixed the problem by saving the address/port info before deleting netbuf:


lwip_recvfrom(...
//INSERTION-->
  struct ip_addr      saved_addr = { 0 };
  u16_t               saved_port = 0;
//<--INSERTION
  ...
      if ((sock->conn->type == NETCONN_TCP) && (buflen - copylen > 0)) {
        sock->lastdata = buf;
        sock->lastoffset += copylen;
        LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_recvfrom: lastdata now netbuf=%p\n", (void*)buf));
      } else {
//INSERTION-->
        /* save addr before deleting buf */
        if (from && fromlen) {
          addr = netbuf_fromaddr(buf);
          if (addr) {
            saved_addr = *addr;
          }
          saved_port = netbuf_fromport(buf);
        }
        /* delete buf */
//<--INSERTION
        sock->lastdata = NULL;
        sock->lastoffset = 0;
        LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_recvfrom: deleting netbuf=%p\n", (void*)buf));
        netbuf_delete(buf);
//INSERTION-->
        buf = NULL;
//<--INSERTION
      }
  ...
  /* Check to see from where the data was.*/
  if (from && fromlen) {
    struct sockaddr_in sin;

    if (netconn_type(sock->conn) == NETCONN_TCP) {
      addr = (struct ip_addr*)&(sin.sin_addr.s_addr);
      netconn_getaddr(sock->conn, addr, &port, 0);
    } else {
//CHANGE-->
      if (buf) {
        addr = netbuf_fromaddr(buf);
        port = netbuf_fromport(buf);
      } else {
        addr = &saved_addr;
        port = saved_port;
      }
//<--CHANGE
    }
  ...
  and the same change in #if SOCKETS_DEBUG section
  ...
}


This is just a quick fix also proving that the root of the problem is here. However it might be possible to give more sophisticated fix (e.g. if I knew that buf would always be disposed in the loop or when to set "done" to 1 or using "mem" instead of "buf", etc.).

Tamas Somogyi <tsomogyi>

 

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

Attached Files
file #17011:  lwip_recvfrom.txt added by fbernon (5KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by fbernon (Posted a comment)
  • -email is unavailable- added by tsomogyi (Submitted the item)
  • -email is unavailable- added by tsomogyi
  •  

    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.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-12-10 fbernon StatusConfirmed Fixed
        Open/ClosedOpen Closed
        Planned Release 1.3.1
    2008-12-09 fbernon Attached File- Added lwip_recvfrom.txt, #17011
    2008-12-09 fbernon StatusNone Confirmed
        Assigned toNone fbernon
    2008-12-09 tsomogyi Carbon-Copy- Added tsomogyi

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code