fixed race conditions in assigning netconn->last_err (fixed bugs #38121 and #37676)

CHANGELOG

@@ -216,6 +216,10 @@ HISTORY
 
   ++ Bugfixes:
 
+  2015-03-19: Simon Goldschmidt
+  * api.h, tcpip.h, api_lib.c, api_msg.c: fixed race conditions in assigning
+    netconn->last_err (fixed bugs #38121 and #37676)
+
   2015-03-09: Simon Goldschmidt
   * ip4.c: fixed the IPv4 part of bug #43904 (ip_route() must detect linkup status)
 

src/api/api_lib.c

@@ -181,7 +181,6 @@ netconn_getaddr(struct netconn *conn, ip_addr_t *addr, u16_t *port, u8_t local)
 #endif /* LWIP_MPU_COMPATIBLE */
   API_MSG_VAR_FREE(msg);
 
-  NETCONN_SET_SAFE_ERR(conn, err);
   return err;
 }
 
@@ -215,7 +214,6 @@ netconn_bind(struct netconn *conn, const ip_addr_t *addr, u16_t port)
   TCPIP_APIMSG(&API_MSG_VAR_REF(msg), lwip_netconn_do_bind, err);
   API_MSG_VAR_FREE(msg);
 
-  NETCONN_SET_SAFE_ERR(conn, err);
   return err;
 }
 
@@ -247,7 +245,6 @@ netconn_connect(struct netconn *conn, const ip_addr_t *addr, u16_t port)
   TCPIP_APIMSG(&API_MSG_VAR_REF(msg), lwip_netconn_do_connect, err);
   API_MSG_VAR_FREE(msg);
 
-  NETCONN_SET_SAFE_ERR(conn, err);
   return err;
 }
 
@@ -270,7 +267,6 @@ netconn_disconnect(struct netconn *conn)
   TCPIP_APIMSG(&API_MSG_VAR_REF(msg), lwip_netconn_do_disconnect, err);
   API_MSG_VAR_FREE(msg);
 
-  NETCONN_SET_SAFE_ERR(conn, err);
   return err;
 }
 
@@ -302,7 +298,6 @@ netconn_listen_with_backlog(struct netconn *conn, u8_t backlog)
   TCPIP_APIMSG(&API_MSG_VAR_REF(msg), lwip_netconn_do_listen, err);
   API_MSG_VAR_FREE(msg);
 
-  NETCONN_SET_SAFE_ERR(conn, err);
   return err;
 #else /* LWIP_TCP */
   LWIP_UNUSED_ARG(conn);
@@ -343,7 +338,6 @@ netconn_accept(struct netconn *conn, struct netconn **new_conn)
 
 #if LWIP_SO_RCVTIMEO
   if (sys_arch_mbox_fetch(&conn->acceptmbox, (void **)&newconn, conn->recv_timeout) == SYS_ARCH_TIMEOUT) {
-    NETCONN_SET_SAFE_ERR(conn, ERR_TIMEOUT);
     return ERR_TIMEOUT;
   }
 #else
@@ -354,6 +348,9 @@ netconn_accept(struct netconn *conn, struct netconn **new_conn)
 
   if (newconn == NULL) {
     /* connection has been aborted */
+    /* in this special case, we set the netconn error from application thread, as
+       on a ready-to-accept listening netconn, there should not be anything running
+       in tcpip_thread */
     NETCONN_SET_SAFE_ERR(conn, ERR_ABRT);
     return ERR_ABRT;
   }
@@ -422,7 +419,6 @@ netconn_recv_data(struct netconn *conn, void **new_buf)
 
 #if LWIP_SO_RCVTIMEO
   if (sys_arch_mbox_fetch(&conn->recvmbox, &buf, conn->recv_timeout) == SYS_ARCH_TIMEOUT) {
-    NETCONN_SET_SAFE_ERR(conn, ERR_TIMEOUT);
     return ERR_TIMEOUT;
   }
 #else
@@ -533,7 +529,6 @@ netconn_recv(struct netconn *conn, struct netbuf **new_buf)
 
     buf = (struct netbuf *)memp_malloc(MEMP_NETBUF);
     if (buf == NULL) {
-      NETCONN_SET_SAFE_ERR(conn, ERR_MEM);
       return ERR_MEM;
     }
 
@@ -639,7 +634,6 @@ netconn_send(struct netconn *conn, struct netbuf *buf)
   TCPIP_APIMSG(&API_MSG_VAR_REF(msg), lwip_netconn_do_send, err);
   API_MSG_VAR_FREE(msg);
 
-  NETCONN_SET_SAFE_ERR(conn, err);
   return err;
 }
 
@@ -711,7 +705,6 @@ netconn_write_partly(struct netconn *conn, const void *dataptr, size_t size,
   }
   API_MSG_VAR_FREE(msg);
 
-  NETCONN_SET_SAFE_ERR(conn, err);
   return err;
 }
 
@@ -745,7 +738,6 @@ netconn_close_shutdown(struct netconn *conn, u8_t how)
   TCPIP_APIMSG(&API_MSG_VAR_REF(msg), lwip_netconn_do_close, err);
   API_MSG_VAR_FREE(msg);
 
-  NETCONN_SET_SAFE_ERR(conn, err);
   return err;
 }
 
@@ -812,7 +804,6 @@ netconn_join_leave_group(struct netconn *conn,
   TCPIP_APIMSG(&API_MSG_VAR_REF(msg), lwip_netconn_do_join_leave_group, err);
   API_MSG_VAR_FREE(msg);
 
-  NETCONN_SET_SAFE_ERR(conn, err);
   return err;
 }
 #endif /* LWIP_IGMP || (LWIP_IPV6 && LWIP_IPV6_MLD) */

src/api/api_msg.c

@@ -251,7 +251,9 @@ recv_tcp(void *arg, struct tcp_pcb *pcb, struct pbuf *p, err_t err)
      (data is already ACKed) */
 
   /* don't overwrite fatal errors! */
-  NETCONN_SET_SAFE_ERR(conn, err);
+  if (err != ERR_OK) {
+    NETCONN_SET_SAFE_ERR(conn, err);
+  }
 
   if (p != NULL) {
     len = p->tot_len;
@@ -416,6 +418,7 @@ err_tcp(void *arg, err_t err)
       LWIP_ASSERT("inavlid op_completed_sem", op_completed_sem != SYS_SEM_NULL);
       conn->current_msg = NULL;
       /* wake up the waiting task */
+      NETCONN_SET_SAFE_ERR(conn, err);
       sys_sem_signal(op_completed_sem);
     }
   } else {
@@ -934,6 +937,7 @@ lwip_netconn_do_close_internal(struct netconn *conn  WRITE_DELAYED_PARAM)
         API_EVENT(conn, NETCONN_EVT_SENDPLUS, 0);
       }
     }
+    NETCONN_SET_SAFE_ERR(conn, err);
 #if LWIP_TCPIP_CORE_LOCKING
     if (delayed)
 #endif
@@ -988,6 +992,7 @@ lwip_netconn_do_delconn(struct api_msg_msg *msg)
       msg->conn->current_msg = NULL;
       msg->conn->write_offset = 0;
       msg->conn->state = NETCONN_NONE;
+      NETCONN_SET_SAFE_ERR(msg->conn, ERR_CLSD);
       sys_sem_signal(op_completed_sem);
     }
   }
@@ -1140,9 +1145,7 @@ lwip_netconn_do_connected(void *arg, struct tcp_pcb *pcb, err_t err)
     (!was_blocking && op_completed_sem == NULL));
   conn->current_msg = NULL;
   conn->state = NETCONN_NONE;
-  if (!was_blocking) {
+  NETCONN_SET_SAFE_ERR(conn, ERR_OK);
-    NETCONN_SET_SAFE_ERR(conn, ERR_OK);
-  }
   API_EVENT(conn, NETCONN_EVT_SENDPLUS, 0);
 
   if (was_blocking) {
@@ -1539,6 +1542,7 @@ err_mem:
     conn->current_msg->err = err;
     conn->current_msg = NULL;
     conn->state = NETCONN_NONE;
+    NETCONN_SET_SAFE_ERR(conn, err);
 #if LWIP_TCPIP_CORE_LOCKING
     if (delayed)
 #endif
@@ -1704,6 +1708,7 @@ lwip_netconn_do_close(struct api_msg_msg *msg)
         msg->conn->current_msg = NULL;
         msg->conn->write_offset = 0;
         msg->conn->state = NETCONN_NONE;
+        NETCONN_SET_SAFE_ERR(msg->conn, ERR_CLSD);
         sys_sem_signal(op_completed_sem);
       } else {
         LWIP_ASSERT("msg->msg.sd.shut == NETCONN_SHUT_RD", msg->msg.sd.shut == NETCONN_SHUT_RD);
@@ -1742,7 +1747,8 @@ lwip_netconn_do_close(struct api_msg_msg *msg)
   {
     msg->err = ERR_CONN;
   }
-  sys_sem_signal(LWIP_API_MSG_SEM(msg));
+  NETCONN_SET_SAFE_ERR(msg->conn, msg->err);
+  TCPIP_APIMSG_ACK(msg);
 }
 
 #if LWIP_IGMP || (LWIP_IPV6 && LWIP_IPV6_MLD)

src/include/lwip/api.h

@@ -235,14 +235,14 @@ struct netconn {
                          }
 
 /** Set conn->last_err to err but don't overwrite fatal errors */
-#define NETCONN_SET_SAFE_ERR(conn, err) do { \
+#define NETCONN_SET_SAFE_ERR(conn, err) do { if ((conn) != NULL) { \
   SYS_ARCH_DECL_PROTECT(lev); \
   SYS_ARCH_PROTECT(lev); \
   if (!ERR_IS_FATAL((conn)->last_err)) { \
     (conn)->last_err = err; \
   } \
   SYS_ARCH_UNPROTECT(lev); \
-} while(0);
+}} while(0);
 
 /* Network connection functions: */
 #define netconn_new(t)                  netconn_new_with_proto_and_callback(t, 0, NULL)

src/include/lwip/tcpip.h

@@ -81,7 +81,7 @@ extern sys_mutex_t lock_tcpip_core;
   TCPIP_APIMSG_NOERR(m,f); \
   (e) = (m)->msg.err; \
 } while(0)
-#define TCPIP_APIMSG_ACK(m)
+#define TCPIP_APIMSG_ACK(m)   NETCONN_SET_SAFE_ERR((m)->conn, (m)->err)
 #define TCPIP_NETIFAPI(m)     tcpip_netifapi_lock(m)
 #define TCPIP_NETIFAPI_ACK(m)
 #define TCPIP_PPPAPI(m)       tcpip_pppapi_lock(m)
@@ -91,7 +91,7 @@ extern sys_mutex_t lock_tcpip_core;
 #define UNLOCK_TCPIP_CORE()
 #define TCPIP_APIMSG_NOERR(m,f) do { (m)->function = f; tcpip_apimsg(m); } while(0)
 #define TCPIP_APIMSG(m,f,e)   do { (m)->function = f; (e) = tcpip_apimsg(m); } while(0)
-#define TCPIP_APIMSG_ACK(m)   sys_sem_signal(LWIP_API_MSG_SEM(m))
+#define TCPIP_APIMSG_ACK(m)   do { NETCONN_SET_SAFE_ERR((m)->conn, (m)->err); sys_sem_signal(LWIP_API_MSG_SEM(m)); } while(0)
 #define TCPIP_NETIFAPI(m)     tcpip_netifapi(m)
 #define TCPIP_NETIFAPI_ACK(m) sys_sem_signal(&m->sem)
 #define TCPIP_PPPAPI(m)       tcpip_pppapi(m)