From 13075460ea10c2902ea2055d18bbcfa73cec8523 Mon Sep 17 00:00:00 2001 From: goldsimon Date: Sun, 22 Jan 2012 11:18:36 +0100 Subject: [PATCH] fixed bug #35305: pcb may be freed too early on shutdown(WR) --- CHANGELOG | 3 +++ src/core/tcp.c | 30 ++++++++++++++++++------------ src/core/tcp_in.c | 6 ++++++ 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 8caf218f..5e935d19 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -76,6 +76,9 @@ HISTORY ++ Bugfixes: + 2012-01-22: Simon Goldschmidt + * tcp.c, tcp_in.c: fixed bug #35305: pcb may be freed too early on shutdown(WR) + 2012-01-21: Simon Goldschmidt * tcp.c: fixed bug #34636: FIN_WAIT_2 - Incorrect shutdown of TCP pcb diff --git a/src/core/tcp.c b/src/core/tcp.c index 162ca366..9adba7cb 100644 --- a/src/core/tcp.c +++ b/src/core/tcp.c @@ -301,7 +301,9 @@ tcp_close(struct tcp_pcb *pcb) /** * Causes all or part of a full-duplex connection of this PCB to be shut down. - * This doesn't deallocate the PCB! + * This doesn't deallocate the PCB unless shutting down both sides! + * Shutting down both sides is the same as calling tcp_close, so if it succeds, + * the PCB should not be referenced any more. * * @param pcb PCB to shutdown * @param shut_rx shut down receive side if this is != 0 @@ -316,28 +318,32 @@ tcp_shutdown(struct tcp_pcb *pcb, int shut_rx, int shut_tx) return ERR_CONN; } if (shut_rx) { - /* shut down the receive side: free buffered data... */ + /* shut down the receive side: set a flag not to receive any more data... */ + pcb->flags |= TF_RXCLOSED; + if (shut_tx) { + /* shutting down the tx AND rx side is the same as closing for the raw API */ + return tcp_close_shutdown(pcb, 1); + } + /* ... and free buffered data */ if (pcb->refused_data != NULL) { pbuf_free(pcb->refused_data); pcb->refused_data = NULL; } - /* ... and set a flag not to receive any more data */ - pcb->flags |= TF_RXCLOSED; } if (shut_tx) { /* This can't happen twice since if it succeeds, the pcb's state is changed. Only close in these states as the others directly deallocate the PCB */ switch (pcb->state) { - case SYN_RCVD: - case ESTABLISHED: - case CLOSE_WAIT: - return tcp_close_shutdown(pcb, 0); - default: - /* don't shut down other states */ - break; + case SYN_RCVD: + case ESTABLISHED: + case CLOSE_WAIT: + return tcp_close_shutdown(pcb, shut_rx); + default: + /* Not (yet?) connected, cannot shutdown the TX side as that would bring us + into CLOSED state, where the PCB is deallocated. */ + return ERR_CONN; } } - /* @todo: return another err_t if not in correct state or already shut? */ return ERR_OK; } diff --git a/src/core/tcp_in.c b/src/core/tcp_in.c index 6af07211..8c54c0bc 100644 --- a/src/core/tcp_in.c +++ b/src/core/tcp_in.c @@ -331,6 +331,12 @@ tcp_input(struct pbuf *p, struct netif *inp) } else if (recv_flags & TF_CLOSED) { /* The connection has been closed and we will deallocate the PCB. */ + if (!(pcb->flags & TF_RXCLOSED)) { + /* Connection closed although the application has only shut down the + tx side: call the PCB's err callback and indicate the closure to + ensure the application doesn't continue using 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 {