forked from Minki/linux
tipc: fix a null-ptr-deref in tipc_topsrv_accept
syzbot found a crash in tipc_topsrv_accept:
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
Workqueue: tipc_rcv tipc_topsrv_accept
RIP: 0010:kernel_accept+0x22d/0x350 net/socket.c:3487
Call Trace:
<TASK>
tipc_topsrv_accept+0x197/0x280 net/tipc/topsrv.c:460
process_one_work+0x991/0x1610 kernel/workqueue.c:2289
worker_thread+0x665/0x1080 kernel/workqueue.c:2436
kthread+0x2e4/0x3a0 kernel/kthread.c:376
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:306
It was caused by srv->listener that might be set to null by
tipc_topsrv_stop() in net .exit whereas it's still used in
tipc_topsrv_accept() worker.
srv->listener is protected by srv->idr_lock in tipc_topsrv_stop(), so add
a check for srv->listener under srv->idr_lock in tipc_topsrv_accept() to
avoid the null-ptr-deref. To ensure the lsock is not released during the
tipc_topsrv_accept(), move sock_release() after tipc_topsrv_work_stop()
where it's waiting until the tipc_topsrv_accept worker to be done.
Note that sk_callback_lock is used to protect sk->sk_user_data instead of
srv->listener, and it should check srv in tipc_topsrv_listener_data_ready()
instead. This also ensures that no more tipc_topsrv_accept worker will be
started after tipc_conn_close() is called in tipc_topsrv_stop() where it
sets sk->sk_user_data to null.
Fixes: 0ef897be12
("tipc: separate topology server listener socket from subcsriber sockets")
Reported-by: syzbot+c5ce866a8d30f4be0651@syzkaller.appspotmail.com
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Jon Maloy <jmaloy@redhat.com>
Link: https://lore.kernel.org/r/4eee264380c409c61c6451af1059b7fb271a7e7b.1666120790.git.lucien.xin@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
parent
6d36c728bc
commit
82cb4e4612
@ -450,12 +450,19 @@ static void tipc_conn_data_ready(struct sock *sk)
|
||||
static void tipc_topsrv_accept(struct work_struct *work)
|
||||
{
|
||||
struct tipc_topsrv *srv = container_of(work, struct tipc_topsrv, awork);
|
||||
struct socket *lsock = srv->listener;
|
||||
struct socket *newsock;
|
||||
struct socket *newsock, *lsock;
|
||||
struct tipc_conn *con;
|
||||
struct sock *newsk;
|
||||
int ret;
|
||||
|
||||
spin_lock_bh(&srv->idr_lock);
|
||||
if (!srv->listener) {
|
||||
spin_unlock_bh(&srv->idr_lock);
|
||||
return;
|
||||
}
|
||||
lsock = srv->listener;
|
||||
spin_unlock_bh(&srv->idr_lock);
|
||||
|
||||
while (1) {
|
||||
ret = kernel_accept(lsock, &newsock, O_NONBLOCK);
|
||||
if (ret < 0)
|
||||
@ -489,7 +496,7 @@ static void tipc_topsrv_listener_data_ready(struct sock *sk)
|
||||
|
||||
read_lock_bh(&sk->sk_callback_lock);
|
||||
srv = sk->sk_user_data;
|
||||
if (srv->listener)
|
||||
if (srv)
|
||||
queue_work(srv->rcv_wq, &srv->awork);
|
||||
read_unlock_bh(&sk->sk_callback_lock);
|
||||
}
|
||||
@ -699,8 +706,9 @@ static void tipc_topsrv_stop(struct net *net)
|
||||
__module_get(lsock->sk->sk_prot_creator->owner);
|
||||
srv->listener = NULL;
|
||||
spin_unlock_bh(&srv->idr_lock);
|
||||
sock_release(lsock);
|
||||
|
||||
tipc_topsrv_work_stop(srv);
|
||||
sock_release(lsock);
|
||||
idr_destroy(&srv->conn_idr);
|
||||
kfree(srv);
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user