lwip-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[lwip-devel] [bug #46696] accepts_pending not decreased when TCP_EVENT_A


From: Joel Cunningham
Subject: [lwip-devel] [bug #46696] accepts_pending not decreased when TCP_EVENT_ACCEPT returns error
Date: Wed, 16 Dec 2015 15:37:23 +0000
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0

URL:
  <http://savannah.nongnu.org/bugs/?46696>

                 Summary: accepts_pending not decreased when TCP_EVENT_ACCEPT
returns error
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: jcunningham
            Submitted on: Wed 16 Dec 2015 03:37:22 PM GMT
                Category: TCP
                Severity: 3 - Normal
              Item Group: Faulty Behaviour
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: 
            lwIP version: git head

    _______________________________________________________

Details:

I'm running LwIP master branch and in my product (NO_SYS = 0, sockets/netconn
enabled), I've encountered the following behavior:

With a TCP listener, if a connection in SYN_RCVD receives its ACK, but calling
TCP_EVENT_ACCEPT() returns an error (ERR_MEM in my case) then the new
connection is aborted, but accepts_pending is not decreased on the parent
listener PCB, causing a leak in accepts_pending

In the call tree of tcp_abort(), tcp_pcb_purge() does get called, which
contains logic to decrease accepts_pending on the listener PCB, but only if
the PCB is in SYN_RCVD and in our case, we advance the state to ESTABLISHED
right before calling TCP_EVENT_ACCEPT

The referenced code is in tcp_in.c, tcp_process()


case SYN_RCVD:
    if (flags & TCP_ACK) {
      /* expected ACK number? */
      if (TCP_SEQ_BETWEEN(ackno, pcb->lastack+1, pcb->snd_nxt)) {
        pcb->state = ESTABLISHED;
        LWIP_DEBUGF(TCP_DEBUG, ("TCP connection established %"U16_F" ->
%"U16_F".\n", inseg.tcphdr->src, inseg.tcphdr->dest));
#if LWIP_CALLBACK_API
        LWIP_ASSERT("pcb->accept != NULL", pcb->accept != NULL);
#endif
        /* Call the accept function. */
        TCP_EVENT_ACCEPT(pcb, ERR_OK, err);
        if (err != ERR_OK) {
          /* If the accept function returns with an error, we abort
           * the connection. */
          /* Already aborted? */
          if (err != ERR_ABRT) {
            tcp_abort(pcb);
          }
          return ERR_ABRT;
        }


I had a couple of ideas:

0 Don't advance the state to ESTABLISHED until after calling TCP_EVENT_ACCEPT
0 Revert the state to SYN_RVCD before calling tcp_abort() in the error case
0 Manually perform the booking keeping on the listener PCB in the error case
of SYN_RCVD

I'm leaning towards #2 as that doesn't change anything when TCP_EVENT_ACCEPT()
is called and doesn't introduce duplicated code




    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?46696>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]