local socket: fix accept used after free

GUIDINGLI · xiaoxiang781216 · commit 00c080174379 · 2023-09-24T10:42:35.000+08:00
==1729315==ERROR: AddressSanitizer: heap-use-after-free on address 0xf0501d60 at pc 0x032ffe43 bp 0xef4ed158 sp 0xef4ed148 READ of size 2 at 0xf0501d60 thread T0 #0 0x32ffe42 in nxsem_wait semaphore/sem_wait.c:94 #1 0x3548cf5 in _net_timedwait utils/net_lock.c:97 #2 0x3548f48 in net_sem_timedwait utils/net_lock.c:236 apache#3 0x3548f8c in net_sem_wait utils/net_lock.c:318 apache#4 0x350124d in local_accept local/local_accept.c:246 apache#5 0x3492719 in psock_accept socket/accept.c:149 apache#6 0x3492bcc in accept4 socket/accept.c:280 apache#7 0x662dc04 in accept net/lib_accept.c:50 apache#8 0x55c81ab in kvdb_loop kvdb/server.c:415 apache#9 0x55c860a in kvdbd_main kvdb/server.c:458 apache#10 0x33d968b in nxtask_startup sched/task_startup.c:70 apache#11 0x32ec039 in nxtask_start task/task_start.c:134 apache#12 0x34109be in pre_start sim/sim_initialstate.c:52 0xf0501d60 is located 288 bytes inside of 420-byte region [0xf0501c40,0xf0501de4) freed by thread T0 here: #0 0xf7aa6a3f in __interceptor_free ../../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:127 #1 0x73aa06e in host_free sim/posix/sim_hostmemory.c:192 #2 0x34131d6 in mm_free sim/sim_heap.c:230 apache#3 0x3409388 in free umm_heap/umm_free.c:49 apache#4 0x35631f3 in local_free local/local_conn.c:225 apache#5 0x3563f75 in local_release local/local_release.c:129 apache#6 0x34f5a32 in local_close local/local_sockif.c:785 apache#7 0x3496ee8 in psock_close socket/net_close.c:102 apache#8 0x36500bc in sock_file_close socket/socket.c:115 apache#9 0x3635f6c in file_close vfs/fs_close.c:74 apache#10 0x3632439 in nx_close_from_tcb inode/fs_files.c:670 apache#11 0x36324f3 in nx_close inode/fs_files.c:697 apache#12 0x3632557 in close inode/fs_files.c:735 apache#13 0x55be289 in property_set_ kvdb/client.c:210 apache#14 0x55c0309 in property_set_int32_ kvdb/common.c:226 apache#15 0x55c03f5 in property_set_int32_oneway kvdb/common.c:236 Signed-off-by: ligd <liguiding1@xiaomi.com>
diff --git a/net/local/local.h b/net/local/local.h
@@ -223,6 +223,40 @@ FAR struct local_conn_s *local_alloc(void);
 
 void local_free(FAR struct local_conn_s *conn);
 
+/****************************************************************************
+ * Name: local_addref
+ *
+ * Description:
+ *   Increment the reference count on the underlying connection structure.
+ *
+ * Input Parameters:
+ *   conn - Socket structure of the socket whose reference count will be
+ *           incremented.
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void local_addref(FAR struct local_conn_s *conn);
+
+/****************************************************************************
+ * Name: local_subref
+ *
+ * Description:
+ *   Subtract the reference count on the underlying connection structure.
+ *
+ * Input Parameters:
+ *   conn - Socket structure of the socket whose reference count will be
+ *           incremented.
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void local_subref(FAR struct local_conn_s *conn);
+
 /****************************************************************************
  * Name: local_nextconn
  *
diff --git a/net/local/local_accept.c b/net/local/local_accept.c
@@ -144,6 +144,8 @@ int local_accept(FAR struct socket *psock, FAR struct sockaddr *addr,
           client = container_of(waiter, struct local_conn_s,
                                 u.client.lc_waiter);
 
+          local_addref(client);
+
           /* Decrement the number of pending clients */
 
           DEBUGASSERT(server->u.server.lc_pending > 0);
@@ -163,7 +165,8 @@ int local_accept(FAR struct socket *psock, FAR struct sockaddr *addr,
             {
               /* Initialize the new connection structure */
 
-              conn->lc_crefs  = 1;
+              local_addref(conn);
+
               conn->lc_proto  = SOCK_STREAM;
               conn->lc_type   = LOCAL_TYPE_PATHNAME;
               conn->lc_state  = LOCAL_STATE_CONNECTED;
@@ -246,6 +249,8 @@ int local_accept(FAR struct socket *psock, FAR struct sockaddr *addr,
               ret = net_sem_wait(&client->lc_donesem);
             }
 
+          local_subref(client);
+
           return ret;
         }
 
diff --git a/net/local/local_conn.c b/net/local/local_conn.c
@@ -225,4 +225,51 @@ void local_free(FAR struct local_conn_s *conn)
   kmm_free(conn);
 }
 
+/****************************************************************************
+ * Name: local_addref
+ *
+ * Description:
+ *   Increment the reference count on the underlying connection structure.
+ *
+ * Input Parameters:
+ *   psock - Socket structure of the socket whose reference count will be
+ *           incremented.
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void local_addref(FAR struct local_conn_s *conn)
+{
+  DEBUGASSERT(conn->lc_crefs >= 0 && conn->lc_crefs < 255);
+  conn->lc_crefs++;
+}
+
+/****************************************************************************
+ * Name: local_subref
+ *
+ * Description:
+ *   Subtract the reference count on the underlying connection structure.
+ *
+ * Input Parameters:
+ *   psock - Socket structure of the socket whose reference count will be
+ *           incremented.
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void local_subref(FAR struct local_conn_s *conn)
+{
+  DEBUGASSERT(conn->lc_crefs > 0 && conn->lc_crefs < 255);
+
+  conn->lc_crefs--;
+  if (conn->lc_crefs == 0)
+    {
+      local_release(conn);
+    }
+}
+
 #endif /* CONFIG_NET && CONFIG_NET_LOCAL */
diff --git a/net/local/local_sockif.c b/net/local/local_sockif.c
@@ -49,7 +49,7 @@
 
 static int        local_setup(FAR struct socket *psock);
 static sockcaps_t local_sockcaps(FAR struct socket *psock);
-static void       local_addref(FAR struct socket *psock);
+static void       local_sockaddref(FAR struct socket *psock);
 static int        local_bind(FAR struct socket *psock,
                     FAR const struct sockaddr *addr, socklen_t addrlen);
 static int        local_getsockname(FAR struct socket *psock,
@@ -80,7 +80,7 @@ const struct sock_intf_s g_local_sockif =
 {
   local_setup,       /* si_setup */
   local_sockcaps,    /* si_sockcaps */
-  local_addref,      /* si_addref */
+  local_sockaddref,  /* si_addref */
   local_bind,        /* si_bind */
   local_getsockname, /* si_getsockname */
   local_getpeername, /* si_getpeername */
@@ -137,8 +137,7 @@ static int local_sockif_alloc(FAR struct socket *psock)
    * count will be incremented only if the socket is dup'ed
    */
 
-  DEBUGASSERT(conn->lc_crefs == 0);
-  conn->lc_crefs = 1;
+  local_addref(conn);
 
   /* Save the pre-allocated connection in the socket structure */
 
@@ -243,7 +242,7 @@ static sockcaps_t local_sockcaps(FAR struct socket *psock)
 }
 
 /****************************************************************************
- * Name: local_addref
+ * Name: local_sockaddref
  *
  * Description:
  *   Increment the reference count on the underlying connection structure.
@@ -257,15 +256,10 @@ static sockcaps_t local_sockcaps(FAR struct socket *psock)
  *
  ****************************************************************************/
 
-static void local_addref(FAR struct socket *psock)
+static void local_sockaddref(FAR struct socket *psock)
 {
-  FAR struct local_conn_s *conn;
-
   DEBUGASSERT(psock->s_domain == PF_LOCAL);
-
-  conn = psock->s_conn;
-  DEBUGASSERT(conn->lc_crefs > 0 && conn->lc_crefs < 255);
-  conn->lc_crefs++;
+  local_addref(psock->s_conn);
 }
 
 /****************************************************************************
@@ -773,23 +767,11 @@ static int local_close(FAR struct socket *psock)
 #endif
       case SOCK_CTRL:
         {
-          FAR struct local_conn_s *conn = psock->s_conn;
-
           /* Is this the last reference to the connection structure (there
            * could be more if the socket was dup'ed).
            */
 
-          if (conn->lc_crefs <= 1)
-            {
-              conn->lc_crefs = 0;
-              local_release(conn);
-            }
-          else
-           {
-             /* No.. Just decrement the reference count */
-
-             conn->lc_crefs--;
-           }
+          local_subref(psock->s_conn);
 
           return OK;
         }
