[lwip-devel] [bug #35305] Too early free of TCP pcb while closing the socket

[Amir Shalem]

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

                 Summary: Too early free of TCP pcb while closing the socket
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: amirshalem
            Submitted on: Fri 13 Jan 2012 05:15:51 PM GMT
                Category: TCP
                Severity: 3 - Normal
              Item Group: Crash Error
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: 
            lwIP version: CVS Head

    _______________________________________________________

Details:

Hey
I found an early free condition of a TCP pcb while the connection is closing,
and the netconn is still referencing the PCB.

- Assume pcb->state = ESTABLISHED.

1. Other side sends us FIN.
2. Our pcb moves to state = CLOSE_WAIT.
3. Call TCP_EVENT_CLOSED(pcb, err);

- api_msg gets an empty tcp_recv(), and passes the application layer an empty
recv()
- The application realize the connection is dead, and calls a
shutdown(SHUT_WR), and afterwards a close()

4. Our application calls shutdown(SHUT_WR)
5. Causes a do_close_internal(SHUT_WR) --> tcp_shutdown(pcb, shut_tx=1) -->
tcp_close_shutdown(pcb, 0) 
6. Which changes our state to LAST_ACK (we are in CLOSE_WAIT).
7. Sends a fin to the other side as-well.

- ACK arrives to our FIN.

8. tcp_process() handles it, and sets TF_CLOSED.
9. tcp_input() handles the TF_CLOSED, and free our PCB - this is the first
free.

But the api_msg conn is still referencing the PCB, which can cause all sorts
of problems.

Usually, if no new socket is created nothing bad happens.
Because if the application closes the socket --> tcp_close_shutdown() -->
state=LAST_ACK and nothing bad happens.

But, if a new connection arrives between the application close()
to the "ACK for our fin", there will a double-reference for this PCB -->
hell.

I'm not sure what is the fix for this bug, maybe adding a 

      } else if (recv_flags & TF_CLOSED) {
        /* The connection has been closed and we will deallocate the
           PCB. */
+       TCP_EVENT_ERR(pcb->errf, pcb->callback_arg, ERR_CLSD);
        tcp_pcb_remove(&tcp_active_pcbs, pcb);
        memp_free(MEMP_TCP_PCB, pcb);
      } else {

while handling the TF_CLOSED? (same way of handling TF_RESET)

What do you think?

Amir.




    _______________________________________________________

Reply to this item at:

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

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