Skip to content

Commit 52565a9

Browse files
Nevsorkuba-moo
authored andcommitted
net: kcm: Fix race condition in kcm_unattach()
syzbot found a race condition when kcm_unattach(psock) and kcm_release(kcm) are executed at the same time. kcm_unattach() is missing a check of the flag kcm->tx_stopped before calling queue_work(). If the kcm has a reserved psock, kcm_unattach() might get executed between cancel_work_sync() and unreserve_psock() in kcm_release(), requeuing kcm->tx_work right before kcm gets freed in kcm_done(). Remove kcm->tx_stopped and replace it by the less error-prone disable_work_sync(). Fixes: ab7ac4e ("kcm: Kernel Connection Multiplexor module") Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=e62c9db591c30e174662 Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=d199b52665b6c3069b94 Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=be6b1fdfeae512726b4e Signed-off-by: Sven Stegemann <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 1c75609 commit 52565a9

File tree

2 files changed

+2
-9
lines changed

2 files changed

+2
-9
lines changed

include/net/kcm.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ struct kcm_sock {
7171
struct list_head wait_psock_list;
7272
struct sk_buff *seq_skb;
7373
struct mutex tx_mutex;
74-
u32 tx_stopped : 1;
7574

7675
/* Don't use bit fields here, these are set under different locks */
7776
bool tx_wait;

net/kcm/kcmsock.c

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ static void psock_write_space(struct sock *sk)
430430

431431
/* Check if the socket is reserved so someone is waiting for sending. */
432432
kcm = psock->tx_kcm;
433-
if (kcm && !unlikely(kcm->tx_stopped))
433+
if (kcm)
434434
queue_work(kcm_wq, &kcm->tx_work);
435435

436436
spin_unlock_bh(&mux->lock);
@@ -1693,12 +1693,6 @@ static int kcm_release(struct socket *sock)
16931693
*/
16941694
__skb_queue_purge(&sk->sk_write_queue);
16951695

1696-
/* Set tx_stopped. This is checked when psock is bound to a kcm and we
1697-
* get a writespace callback. This prevents further work being queued
1698-
* from the callback (unbinding the psock occurs after canceling work.
1699-
*/
1700-
kcm->tx_stopped = 1;
1701-
17021696
release_sock(sk);
17031697

17041698
spin_lock_bh(&mux->lock);
@@ -1714,7 +1708,7 @@ static int kcm_release(struct socket *sock)
17141708
/* Cancel work. After this point there should be no outside references
17151709
* to the kcm socket.
17161710
*/
1717-
cancel_work_sync(&kcm->tx_work);
1711+
disable_work_sync(&kcm->tx_work);
17181712

17191713
lock_sock(sk);
17201714
psock = kcm->tx_psock;

0 commit comments

Comments
 (0)