You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2023/09/24 02:42:41 UTC
[nuttx] branch master updated: local socket: fix accept used after free
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 00c0801743 local socket: fix accept used after free
00c0801743 is described below
commit 00c080174379a31b74f9b42abf20a29effd7b09f
Author: ligd <li...@xiaomi.com>
AuthorDate: Fri Sep 22 13:33:42 2023 +0800
local socket: fix accept used after free
==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
#3 0x3548f8c in net_sem_wait utils/net_lock.c:318
#4 0x350124d in local_accept local/local_accept.c:246
#5 0x3492719 in psock_accept socket/accept.c:149
#6 0x3492bcc in accept4 socket/accept.c:280
#7 0x662dc04 in accept net/lib_accept.c:50
#8 0x55c81ab in kvdb_loop kvdb/server.c:415
#9 0x55c860a in kvdbd_main kvdb/server.c:458
#10 0x33d968b in nxtask_startup sched/task_startup.c:70
#11 0x32ec039 in nxtask_start task/task_start.c:134
#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
#3 0x3409388 in free umm_heap/umm_free.c:49
#4 0x35631f3 in local_free local/local_conn.c:225
#5 0x3563f75 in local_release local/local_release.c:129
#6 0x34f5a32 in local_close local/local_sockif.c:785
#7 0x3496ee8 in psock_close socket/net_close.c:102
#8 0x36500bc in sock_file_close socket/socket.c:115
#9 0x3635f6c in file_close vfs/fs_close.c:74
#10 0x3632439 in nx_close_from_tcb inode/fs_files.c:670
#11 0x36324f3 in nx_close inode/fs_files.c:697
#12 0x3632557 in close inode/fs_files.c:735
#13 0x55be289 in property_set_ kvdb/client.c:210
#14 0x55c0309 in property_set_int32_ kvdb/common.c:226
#15 0x55c03f5 in property_set_int32_oneway kvdb/common.c:236
Signed-off-by: ligd <li...@xiaomi.com>
---
net/local/local.h | 34 ++++++++++++++++++++++++++++++++++
net/local/local_accept.c | 7 ++++++-
net/local/local_conn.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
net/local/local_sockif.c | 32 +++++++-------------------------
4 files changed, 94 insertions(+), 26 deletions(-)
diff --git a/net/local/local.h b/net/local/local.h
index cb9c00d5b8..8804c783e1 100644
--- 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
index df727a34d2..9621335c81 100644
--- 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
index 6d8cef1518..7a27c40661 100644
--- 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
index 500b3ee6f0..77b5abb55e 100644
--- 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;
}