From 48ca949fd4d60271f3e50bee945822cd546e592d Mon Sep 17 00:00:00 2001 From: Liu Zhi Fu Date: Tue, 7 Nov 2017 20:10:02 +0800 Subject: [PATCH] lwip: fix socket close crash issue When lwip_close free the socket resource (netconn etc), any work related to these resource (netconn etc) must be aborted. --- components/esp32/lib | 2 +- components/lwip/api/api_msg.c | 63 ++++++++++++++++++++++++++++++----- components/lwip/core/tcp.c | 3 +- components/lwip/core/tcp_in.c | 2 ++ 4 files changed, 58 insertions(+), 12 deletions(-) diff --git a/components/esp32/lib b/components/esp32/lib index 00315e422..4fe99c9fc 160000 --- a/components/esp32/lib +++ b/components/esp32/lib @@ -1 +1 @@ -Subproject commit 00315e422f26713506a864410be449414087af72 +Subproject commit 4fe99c9fc04c35e71f5631b59a2e976d18a93da5 diff --git a/components/lwip/api/api_msg.c b/components/lwip/api/api_msg.c index 5e5b8b2ca..a3f21fb53 100755 --- a/components/lwip/api/api_msg.c +++ b/components/lwip/api/api_msg.c @@ -74,9 +74,21 @@ #define WRITE_DELAYED_PARAM #endif /* LWIP_TCPIP_CORE_LOCKING */ static err_t lwip_netconn_do_writemore(struct netconn *conn WRITE_DELAYED_PARAM); -static err_t lwip_netconn_do_close_internal(struct netconn *conn WRITE_DELAYED_PARAM); + +#if ESP_LWIP +#define SIG_CLOSE_PARAM , bool sig_close +#define SIG_CLOSE_TRUE true +#define SIG_CLOSE_FALSE false +#else +#define SIG_CLOSE_PARAM +#define SIG_CLOSE_TRUE +#define SIG_CLOSE_FALSE #endif +#endif + +static err_t lwip_netconn_do_close_internal(struct netconn *conn WRITE_DELAYED_PARAM SIG_CLOSE_PARAM); + #if LWIP_RAW /** * Receive callback function for RAW netconns. @@ -309,6 +321,7 @@ static err_t poll_tcp(void *arg, struct tcp_pcb *pcb) { struct netconn *conn = (struct netconn *)arg; + bool sig_close = false; LWIP_UNUSED_ARG(pcb); LWIP_ASSERT("conn != NULL", (conn != NULL)); @@ -321,7 +334,10 @@ poll_tcp(void *arg, struct tcp_pcb *pcb) conn->current_msg->msg.sd.polls_left--; } #endif /* !LWIP_SO_SNDTIMEO && !LWIP_SO_LINGER */ - lwip_netconn_do_close_internal(conn WRITE_DELAYED); + /* Delay the netconn close until no one use 'conn' because close frees 'conn'*/ + if (ERR_OK == lwip_netconn_do_close_internal(conn WRITE_DELAYED, SIG_CLOSE_FALSE)) { + sig_close = true; + } } /* @todo: implement connect timeout here? */ @@ -336,6 +352,15 @@ poll_tcp(void *arg, struct tcp_pcb *pcb) } } +#if ESP_LWIP + if (sig_close) { + sys_sem_t *op_completed_sem = LWIP_API_MSG_SEM(conn->current_msg); + conn->current_msg = NULL; + sys_sem_signal(op_completed_sem); + return ERR_ABRT; + } +#endif + return ERR_OK; } @@ -350,6 +375,7 @@ static err_t sent_tcp(void *arg, struct tcp_pcb *pcb, u16_t len) { struct netconn *conn = (struct netconn *)arg; + bool sig_close = false; LWIP_UNUSED_ARG(pcb); LWIP_ASSERT("conn != NULL", (conn != NULL)); @@ -358,7 +384,10 @@ sent_tcp(void *arg, struct tcp_pcb *pcb, u16_t len) if (conn->state == NETCONN_WRITE) { lwip_netconn_do_writemore(conn WRITE_DELAYED); } else if (conn->state == NETCONN_CLOSE) { - lwip_netconn_do_close_internal(conn WRITE_DELAYED); + /* Delay the netconn close until no one use 'conn' because close frees 'conn'*/ + if (ERR_OK == lwip_netconn_do_close_internal(conn WRITE_DELAYED, SIG_CLOSE_FALSE)) { + sig_close = true; + } } /* If the queued byte- or pbuf-count drops below the configured low-water limit, @@ -368,6 +397,15 @@ sent_tcp(void *arg, struct tcp_pcb *pcb, u16_t len) conn->flags &= ~NETCONN_FLAG_CHECK_WRITESPACE; API_EVENT(conn, NETCONN_EVT_SENDPLUS, len); } + +#if ESP_LWIP + if (sig_close) { + sys_sem_t *op_completed_sem = LWIP_API_MSG_SEM(conn->current_msg); + conn->current_msg = NULL; + sys_sem_signal(op_completed_sem); + return ERR_ABRT; + } +#endif } return ERR_OK; @@ -802,7 +840,7 @@ netconn_drain(struct netconn *conn) * [@param delay 1 if called from sent/poll (wake up calling thread on end)] */ static err_t -lwip_netconn_do_close_internal(struct netconn *conn WRITE_DELAYED_PARAM) +lwip_netconn_do_close_internal(struct netconn *conn WRITE_DELAYED_PARAM SIG_CLOSE_PARAM) { err_t err; u8_t shut, shut_rx, shut_tx, close; @@ -947,7 +985,6 @@ lwip_netconn_do_close_internal(struct netconn *conn WRITE_DELAYED_PARAM) /* Closing done (succeeded, non-memory error, nonblocking error or timeout) */ sys_sem_t* op_completed_sem = LWIP_API_MSG_SEM(conn->current_msg); conn->current_msg->err = err; - conn->current_msg = NULL; conn->state = NETCONN_NONE; if (err == ERR_OK) { if (close) { @@ -970,7 +1007,15 @@ lwip_netconn_do_close_internal(struct netconn *conn WRITE_DELAYED_PARAM) #endif { /* wake up the application task */ +#if ESP_LWIP + if (sig_close) { + conn->current_msg = NULL; + sys_sem_signal(op_completed_sem); + } +#else + conn->current_msg = NULL; sys_sem_signal(op_completed_sem); +#endif } return ERR_OK; } @@ -1071,7 +1116,7 @@ lwip_netconn_do_delconn(void *m) msg->msg.sd.shut = NETCONN_SHUT_RDWR; msg->conn->current_msg = msg; #if LWIP_TCPIP_CORE_LOCKING - if (lwip_netconn_do_close_internal(msg->conn, 0) != ERR_OK) { + if (lwip_netconn_do_close_internal(msg->conn, 0, SIG_CLOSE_TRUE) != ERR_OK) { LWIP_ASSERT("state!", msg->conn->state == NETCONN_CLOSE); UNLOCK_TCPIP_CORE(); sys_arch_sem_wait(LWIP_API_MSG_SEM(msg), 0); @@ -1079,7 +1124,7 @@ lwip_netconn_do_delconn(void *m) LWIP_ASSERT("state!", msg->conn->state == NETCONN_NONE); } #else /* LWIP_TCPIP_CORE_LOCKING */ - lwip_netconn_do_close_internal(msg->conn); + lwip_netconn_do_close_internal(msg->conn, SIG_CLOSE_TRUE); #endif /* LWIP_TCPIP_CORE_LOCKING */ /* API_EVENT is called inside lwip_netconn_do_close_internal, before releasing the application thread, so we can return at this point! */ @@ -1815,7 +1860,7 @@ lwip_netconn_do_close(void *m) msg->conn->state = NETCONN_CLOSE; msg->conn->current_msg = msg; #if LWIP_TCPIP_CORE_LOCKING - if (lwip_netconn_do_close_internal(msg->conn, 0) != ERR_OK) { + if (lwip_netconn_do_close_internal(msg->conn, 0, SIG_CLOSE_TRUE) != ERR_OK) { LWIP_ASSERT("state!", msg->conn->state == NETCONN_CLOSE); UNLOCK_TCPIP_CORE(); sys_arch_sem_wait(LWIP_API_MSG_SEM(msg), 0); @@ -1823,7 +1868,7 @@ lwip_netconn_do_close(void *m) LWIP_ASSERT("state!", msg->conn->state == NETCONN_NONE); } #else /* LWIP_TCPIP_CORE_LOCKING */ - lwip_netconn_do_close_internal(msg->conn); + lwip_netconn_do_close_internal(msg->conn, SIG_CLOSE_TRUE); #endif /* LWIP_TCPIP_CORE_LOCKING */ /* for tcp netconns, lwip_netconn_do_close_internal ACKs the message */ return; diff --git a/components/lwip/core/tcp.c b/components/lwip/core/tcp.c index 7d16fce5b..920523e45 100755 --- a/components/lwip/core/tcp.c +++ b/components/lwip/core/tcp.c @@ -405,6 +405,7 @@ tcp_abandon(struct tcp_pcb *pcb, int reset) #if TCP_QUEUE_OOSEQ if (pcb->ooseq != NULL) { tcp_segs_free(pcb->ooseq); + pcb->ooseq = NULL; } #endif /* TCP_QUEUE_OOSEQ */ if (send_rst) { @@ -1248,9 +1249,7 @@ tcp_seg_free(struct tcp_seg *seg) if (seg != NULL) { if (seg->p != NULL) { pbuf_free(seg->p); -#if TCP_DEBUG seg->p = NULL; -#endif /* TCP_DEBUG */ } memp_free(MEMP_TCP_SEG, seg); } diff --git a/components/lwip/core/tcp_in.c b/components/lwip/core/tcp_in.c index f3284233e..f4fc7e337 100755 --- a/components/lwip/core/tcp_in.c +++ b/components/lwip/core/tcp_in.c @@ -721,6 +721,7 @@ tcp_process(struct tcp_pcb *pcb) /* Do different things depending on the TCP state. */ switch (pcb->state) { case SYN_SENT: + if (pcb->unacked) { LWIP_DEBUGF(TCP_INPUT_DEBUG, ("SYN-SENT: ackno %"U32_F" pcb->snd_nxt %"U32_F" unacked %"U32_F"\n", ackno, pcb->snd_nxt, ntohl(pcb->unacked->tcphdr->seqno))); /* received SYN ACK with expected sequence number? */ @@ -752,6 +753,7 @@ tcp_process(struct tcp_pcb *pcb) rseg = pcb->unacked; pcb->unacked = rseg->next; tcp_seg_free(rseg); + } /* If there's nothing left to acknowledge, stop the retransmit timer, otherwise reset it to start again */