Skip to content

Commit

Permalink
local socket: fix accept used after free
Browse files Browse the repository at this point in the history
==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
    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
    #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
    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>
  • Loading branch information
GUIDINGLI authored and xiaoxiang781216 committed Sep 24, 2023
1 parent 13f0051 commit 00c0801
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 26 deletions.
34 changes: 34 additions & 0 deletions net/local/local.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
7 changes: 6 additions & 1 deletion net/local/local_accept.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
47 changes: 47 additions & 0 deletions net/local/local_conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
32 changes: 7 additions & 25 deletions net/local/local_sockif.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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 */

Expand Down Expand Up @@ -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.
Expand All @@ -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);
}

/****************************************************************************
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 00c0801

Please sign in to comment.