Fix mbox thread-safe issue

Fix a mbox free thread-safe issue that can lead crash in sys_mbox_fetch().
espressif · Jan 2, 2019 · e2a24de · e2a24de
1 parent 93de13b
commit e2a24de
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 24 deletions.
diff --git a/src/api/api_lib.c b/src/api/api_lib.c
@@ -147,6 +147,9 @@ netconn_new_with_proto_and_callback(enum netconn_type t, u8_t proto, netconn_cal
       LWIP_ASSERT("conn has no op_completed", sys_sem_valid(&conn->op_completed));
       sys_sem_free(&conn->op_completed);
 #endif /* !LWIP_NETCONN_SEM_PER_THREAD */
+#if ESP_THREAD_SAFE
+      sys_mbox_set_owner(&conn->recvmbox, NULL);
+#endif
       sys_mbox_free(&conn->recvmbox);
       memp_free(MEMP_NETCONN, conn);
       API_MSG_VAR_FREE(msg);

diff --git a/src/api/api_msg.c b/src/api/api_msg.c
@@ -603,6 +603,9 @@ accept_function(void *arg, struct tcp_pcb *newpcb, err_t err)
     tcp_err(pcb, NULL);
     /* remove reference from to the pcb from this netconn */
     newconn->pcb.tcp = NULL;
+#if ESP_THREAD_SAFE
+    sys_mbox_set_owner(&newconn->recvmbox, NULL);
+#endif
     /* no need to drain since we know the recvmbox is empty. */
     sys_mbox_free(&newconn->recvmbox);
     sys_mbox_set_invalid(&newconn->recvmbox);
@@ -778,14 +781,14 @@ netconn_alloc(enum netconn_type t, netconn_callback callback)
 #endif
 
 #if ESP_THREAD_SAFE
-  conn->recvmbox_ref = conn->recvmbox;
   sys_mbox_set_owner(&conn->recvmbox, conn);
-#if LWIP_TCP
-  sys_mbox_set_invalid(&conn->acceptmbox_ref);
-#endif
 #endif
 
 #if LWIP_TCP
+#if ESP_THREAD_SAFE
+  /* Init acceptmbox to NULL because sys_mbox_set_invalid is implemented as empty macro */
+  conn->acceptmbox = NULL;
+#endif
   sys_mbox_set_invalid(&conn->acceptmbox);
 #endif
   conn->state        = NETCONN_NONE;
@@ -828,24 +831,21 @@ void
 netconn_free(struct netconn *conn)
 {
   LWIP_ASSERT("PCB must be deallocated outside this function", conn->pcb.tcp == NULL);
+#if !ESP_THREAD_SAFE
   LWIP_ASSERT("recvmbox must be deallocated before calling this function",
     !sys_mbox_valid(&conn->recvmbox));
+
 #if LWIP_TCP
   LWIP_ASSERT("acceptmbox must be deallocated before calling this function",
     !sys_mbox_valid(&conn->acceptmbox));
 #endif /* LWIP_TCP */
-
-#if ESP_THREAD_SAFE
-  if (conn->recvmbox_ref) {
-    sys_mbox_free(&conn->recvmbox_ref);
-  }
+#else /* !ESP_THREAD_SAFE */
+  sys_mbox_free(&conn->recvmbox);
 
 #if LWIP_TCP
-  if (conn->acceptmbox_ref) {
-    sys_mbox_free(&conn->acceptmbox_ref);
-  }
-#endif
-#endif
+  sys_mbox_free(&conn->acceptmbox);
+#endif /* LWIP TCP */
+#endif /* !ESP_THREAD_SAFE */
 
 #if !LWIP_NETCONN_SEM_PER_THREAD
   sys_sem_free(&conn->op_completed);
@@ -1508,7 +1508,6 @@ lwip_netconn_do_listen(void *m)
               }
               if (msg->err == ERR_OK) {
 #if ESP_THREAD_SAFE
-                msg->conn->acceptmbox_ref = msg->conn->acceptmbox;
                 sys_mbox_set_owner(&msg->conn->acceptmbox, msg->conn);
 #endif
                 msg->conn->state = NETCONN_LISTEN;

diff --git a/src/include/lwip/api.h b/src/include/lwip/api.h
@@ -233,15 +233,6 @@ struct netconn {
       by the application thread */
   sys_mbox_t acceptmbox;
 #endif /* LWIP_TCP */
-
-#if ESP_THREAD_SAFE
-  /** point to the same mbox as recvmbox */
-  sys_mbox_t recvmbox_ref;
-#if LWIP_TCP
-  /** point to the same mbox as acceptmbox */
-  sys_mbox_t acceptmbox_ref;
-#endif
-#endif
   /** only used for socket layer */
 #if LWIP_SOCKET
   int socket;