From 2b1ee0de3af88b5f85c451906a8854fc7b9c21ae Mon Sep 17 00:00:00 2001
From: David Lockyer <david.lockyer@chronos.co.uk>
Date: Mon, 11 Sep 2017 10:29:41 +0100
Subject: [PATCH] Fixed a memory access fault in lwip_select(), when using MPU.

---
 src/api/sockets.c                | 85 +++++++++++++++++-----------------------
 src/include/lwip/opt.h           |  8 ++++
 src/include/lwip/priv/memp_std.h |  4 ++
 src/include/lwip/sockets.h       | 27 +++++++++++++
 4 files changed, 74 insertions(+), 50 deletions(-)

diff --git a/src/api/sockets.c b/src/api/sockets.c
index b763248..83812b1 100644
--- a/src/api/sockets.c
+++ b/src/api/sockets.c
@@ -50,7 +50,6 @@
 
 #include "lwip/sockets.h"
 #include "lwip/api.h"
-#include "lwip/sys.h"
 #include "lwip/igmp.h"
 #include "lwip/inet.h"
 #include "lwip/tcp.h"
@@ -77,6 +76,13 @@
 #define LWIP_NETCONN 0
 #endif
 
+#define API_SELECT_CB_VAR_REF(name)               API_VAR_REF(name)
+#define API_SELECT_CB_VAR_DECLARE(name)           API_VAR_DECLARE(struct lwip_select_cb, name)
+#define API_SELECT_CB_VAR_ALLOC(name)             API_VAR_ALLOC(struct lwip_select_cb, MEMP_SELECT_CB, name, ERR_MEM)
+#define API_SELECT_CB_VAR_ALLOC_RETURN_NULL(name) API_VAR_ALLOC(struct lwip_select_cb, MEMP_SELECT_CB, name, NULL)
+#define API_SELECT_CB_VAR_FREE(name)              API_VAR_FREE(MEMP_SELECT_CB, name)
+
+
 #if LWIP_IPV4
 #define IP4ADDR_PORT_TO_SOCKADDR(sin, ipaddr, port) do { \
       (sin)->sin_len = sizeof(struct sockaddr_in); \
@@ -218,32 +224,6 @@ struct lwip_sock {
   SELWAIT_T select_waiting;
 };
 
-#if LWIP_NETCONN_SEM_PER_THREAD
-#define SELECT_SEM_T        sys_sem_t*
-#define SELECT_SEM_PTR(sem) (sem)
-#else /* LWIP_NETCONN_SEM_PER_THREAD */
-#define SELECT_SEM_T        sys_sem_t
-#define SELECT_SEM_PTR(sem) (&(sem))
-#endif /* LWIP_NETCONN_SEM_PER_THREAD */
-
-/** Description for a task waiting in select */
-struct lwip_select_cb {
-  /** Pointer to the next waiting task */
-  struct lwip_select_cb *next;
-  /** Pointer to the previous waiting task */
-  struct lwip_select_cb *prev;
-  /** readset passed to select */
-  fd_set *readset;
-  /** writeset passed to select */
-  fd_set *writeset;
-  /** unimplemented: exceptset passed to select */
-  fd_set *exceptset;
-  /** don't signal the same semaphore twice: set to 1 when signalled */
-  int sem_signalled;
-  /** semaphore to wake up a task waiting for select */
-  SELECT_SEM_T sem;
-};
-
 /** A struct sockaddr replacement that has the same alignment as sockaddr_in/
  *  sockaddr_in6 if instantiated.
  */
@@ -1375,7 +1355,7 @@ lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset,
   int nready;
   fd_set lreadset, lwriteset, lexceptset;
   u32_t msectimeout;
-  struct lwip_select_cb select_cb;
+  API_SELECT_CB_VAR_DECLARE(select_cb);
   int i;
   int maxfdp2;
 #if LWIP_NETCONN_SEM_PER_THREAD
@@ -1383,6 +1363,8 @@ lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset,
 #endif
   SYS_ARCH_DECL_PROTECT(lev);
 
+  API_SELECT_CB_VAR_ALLOC(select_cb);
+
   LWIP_DEBUGF(SOCKETS_DEBUG, ("lwip_select(%d, %p, %p, %p, tvsec=%"S32_F" tvusec=%"S32_F")\n",
                   maxfdp1, (void *)readset, (void *) writeset, (void *) exceptset,
                   timeout ? (s32_t)timeout->tv_sec : (s32_t)-1,
@@ -1406,18 +1388,19 @@ lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset,
        list is only valid while we are in this function, so it's ok
        to use local variables. */
 
-    select_cb.next = NULL;
-    select_cb.prev = NULL;
-    select_cb.readset = readset;
-    select_cb.writeset = writeset;
-    select_cb.exceptset = exceptset;
-    select_cb.sem_signalled = 0;
+    API_SELECT_CB_VAR_REF(select_cb).next = NULL;
+    API_SELECT_CB_VAR_REF(select_cb).prev = NULL;
+    API_SELECT_CB_VAR_REF(select_cb).readset = readset;
+    API_SELECT_CB_VAR_REF(select_cb).writeset = writeset;
+    API_SELECT_CB_VAR_REF(select_cb).exceptset = exceptset;
+    API_SELECT_CB_VAR_REF(select_cb).sem_signalled = 0;
 #if LWIP_NETCONN_SEM_PER_THREAD
-    select_cb.sem = LWIP_NETCONN_THREAD_SEM_GET();
+    API_SELECT_CB_VAR_REF(select_cb).sem = LWIP_NETCONN_THREAD_SEM_GET();
 #else /* LWIP_NETCONN_SEM_PER_THREAD */
-    if (sys_sem_new(&select_cb.sem, 0) != ERR_OK) {
+    if (sys_sem_new(&API_SELECT_CB_VAR_REF(select_cb).sem, 0) != ERR_OK) {
       /* failed to create semaphore */
       set_errno(ENOMEM);
+      API_SELECT_CB_VAR_FREE(select_cb);
       return -1;
     }
 #endif /* LWIP_NETCONN_SEM_PER_THREAD */
@@ -1426,11 +1409,11 @@ lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset,
     SYS_ARCH_PROTECT(lev);
 
     /* Put this select_cb on top of list */
-    select_cb.next = select_cb_list;
+    API_SELECT_CB_VAR_REF(select_cb).next = select_cb_list;
     if (select_cb_list != NULL) {
-      select_cb_list->prev = &select_cb;
+      select_cb_list->prev = &API_SELECT_CB_VAR_REF(select_cb);
     }
-    select_cb_list = &select_cb;
+    select_cb_list = &API_SELECT_CB_VAR_REF(select_cb);
     /* Increasing this counter tells event_callback that the list has changed. */
     select_cb_ctr++;
 
@@ -1477,7 +1460,7 @@ lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset,
           }
         }
 
-        waitres = sys_arch_sem_wait(SELECT_SEM_PTR(select_cb.sem), msectimeout);
+        waitres = sys_arch_sem_wait(SELECT_SEM_PTR(API_SELECT_CB_VAR_REF(select_cb).sem), msectimeout);
 #if LWIP_NETCONN_SEM_PER_THREAD
         waited = 1;
 #endif
@@ -1507,32 +1490,33 @@ lwip_select(int maxfdp1, fd_set *readset, fd_set *writeset, fd_set *exceptset,
     }
     /* Take us off the list */
     SYS_ARCH_PROTECT(lev);
-    if (select_cb.next != NULL) {
-      select_cb.next->prev = select_cb.prev;
+    if (API_SELECT_CB_VAR_REF(select_cb).next != NULL) {
+      API_SELECT_CB_VAR_REF(select_cb).next->prev = API_SELECT_CB_VAR_REF(select_cb).prev;
     }
-    if (select_cb_list == &select_cb) {
-      LWIP_ASSERT("select_cb.prev == NULL", select_cb.prev == NULL);
-      select_cb_list = select_cb.next;
+    if (select_cb_list == &API_SELECT_CB_VAR_REF(select_cb)) {
+      LWIP_ASSERT("select_cb.prev == NULL", API_SELECT_CB_VAR_REF(select_cb).prev == NULL);
+      select_cb_list = API_SELECT_CB_VAR_REF(select_cb).next;
     } else {
-      LWIP_ASSERT("select_cb.prev != NULL", select_cb.prev != NULL);
-      select_cb.prev->next = select_cb.next;
+      LWIP_ASSERT("select_cb.prev != NULL", API_SELECT_CB_VAR_REF(select_cb).prev != NULL);
+      API_SELECT_CB_VAR_REF(select_cb).prev->next = API_SELECT_CB_VAR_REF(select_cb).next;
     }
     /* Increasing this counter tells event_callback that the list has changed. */
     select_cb_ctr++;
     SYS_ARCH_UNPROTECT(lev);
 
 #if LWIP_NETCONN_SEM_PER_THREAD
-    if (select_cb.sem_signalled && (!waited || (waitres == SYS_ARCH_TIMEOUT))) {
+    if (API_SELECT_CB_VAR_REF(select_cb).sem_signalled && (!waited || (waitres == SYS_ARCH_TIMEOUT))) {
       /* don't leave the thread-local semaphore signalled */
-      sys_arch_sem_wait(select_cb.sem, 1);
+      sys_arch_sem_wait(API_SELECT_CB_VAR_REF(select_cb).sem, 1);
     }
 #else /* LWIP_NETCONN_SEM_PER_THREAD */
-    sys_sem_free(&select_cb.sem);
+    sys_sem_free(&API_SELECT_CB_VAR_REF(select_cb).sem);
 #endif /* LWIP_NETCONN_SEM_PER_THREAD */
 
     if (nready < 0) {
       /* This happens when a socket got closed while waiting */
       set_errno(EBADF);
+      API_SELECT_CB_VAR_FREE(select_cb);
       return -1;
     }
 
@@ -1560,6 +1544,7 @@ return_copy_fdsets:
   if (exceptset) {
     *exceptset = lexceptset;
   }
+  API_SELECT_CB_VAR_FREE(select_cb);
   return nready;
 }
 
diff --git a/src/include/lwip/opt.h b/src/include/lwip/opt.h
index fd459af..01a09f9 100644
--- a/src/include/lwip/opt.h
+++ b/src/include/lwip/opt.h
@@ -455,6 +455,14 @@
 #endif
 
 /**
+ * MEMP_NUM_SELECT_CB: the number of struct lwip_select_cb.
+ * (only needed if you use the sequential API, like api_lib.c)
+ */
+#if !defined MEMP_NUM_SELECT_CB || defined __DOXYGEN__
+#define MEMP_NUM_SELECT_CB              4
+#endif
+
+/**
  * MEMP_NUM_TCPIP_MSG_API: the number of struct tcpip_msg, which are used
  * for callback/timeout API communication.
  * (only needed if you use tcpip.c)
diff --git a/src/include/lwip/priv/memp_std.h b/src/include/lwip/priv/memp_std.h
index ce9fd50..144d965 100644
--- a/src/include/lwip/priv/memp_std.h
+++ b/src/include/lwip/priv/memp_std.h
@@ -64,6 +64,10 @@ LWIP_MEMPOOL(NETBUF,         MEMP_NUM_NETBUF,          sizeof(struct netbuf),
 LWIP_MEMPOOL(NETCONN,        MEMP_NUM_NETCONN,         sizeof(struct netconn),        "NETCONN")
 #endif /* LWIP_NETCONN || LWIP_SOCKET */
 
+#if LWIP_SOCKET
+LWIP_MEMPOOL(SELECT_CB,      MEMP_NUM_SELECT_CB,       sizeof(struct lwip_select_cb), "SELECT_CB")
+#endif /* LWIP_SOCKET */
+
 #if NO_SYS==0
 LWIP_MEMPOOL(TCPIP_MSG_API,  MEMP_NUM_TCPIP_MSG_API,   sizeof(struct tcpip_msg),      "TCPIP_MSG_API")
 #if LWIP_MPU_COMPATIBLE
diff --git a/src/include/lwip/sockets.h b/src/include/lwip/sockets.h
index 2522056..0112fbb 100644
--- a/src/include/lwip/sockets.h
+++ b/src/include/lwip/sockets.h
@@ -47,6 +47,7 @@
 #include "lwip/err.h"
 #include "lwip/inet.h"
 #include "lwip/errno.h"
+#include "lwip/sys.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -159,6 +160,32 @@ struct msghdr {
   int           msg_flags;
 };
 
+#if LWIP_NETCONN_SEM_PER_THREAD
+#define SELECT_SEM_T        sys_sem_t*
+#define SELECT_SEM_PTR(sem) (sem)
+#else /* LWIP_NETCONN_SEM_PER_THREAD */
+#define SELECT_SEM_T        sys_sem_t
+#define SELECT_SEM_PTR(sem) (&(sem))
+#endif /* LWIP_NETCONN_SEM_PER_THREAD */
+
+/** Description for a task waiting in select */
+struct lwip_select_cb {
+  /** Pointer to the next waiting task */
+  struct lwip_select_cb *next;
+  /** Pointer to the previous waiting task */
+  struct lwip_select_cb *prev;
+  /** readset passed to select */
+  fd_set *readset;
+  /** writeset passed to select */
+  fd_set *writeset;
+  /** unimplemented: exceptset passed to select */
+  fd_set *exceptset;
+  /** don't signal the same semaphore twice: set to 1 when signalled */
+  int sem_signalled;
+  /** semaphore to wake up a task waiting for select */
+  SELECT_SEM_T sem;
+};
+
 /* Socket protocol types (TCP/UDP/RAW) */
 #define SOCK_STREAM     1
 #define SOCK_DGRAM      2
-- 
2.7.4

