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

Submitted by:  Tamas Somogyi <tsomogyi>
Submitted on:  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:  Frédéric Bernon <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>
Project MemberIn charge of this item.
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>
Project 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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>

 

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
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    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.5