[PATCH net] tipc: purge cong_links under the socket lock in tipc_release()

[PATCH net] tipc: purge cong_links under the socket lock in tipc_release()

[Jun Yang]

From: Jun Yang <junvyyang@tencent.com>

tipc_release() frees the elements of tsk->cong_links after release_sock(),
i.e. with no lock held:

	tipc_sk_remove(tsk);
	sock_orphan(sk);
	release_sock(sk);
	tipc_dest_list_purge(&tsk->cong_links);	/* no lock */
	tsk->cong_link_cnt = 0;

Every other accessor of that list runs under the socket lock, including
the SOCK_WAKEUP handler in tipc_sk_proto_rcv(), which does
tipc_dest_del(&tsk->cong_links, ...) while holding only sk->sk_lock.slock
via tipc_sk_rcv()'s spin_trylock_bh().  Because the purge never acquires
that spinlock, it provides no mutual exclusion against the wakeup path.

A SOCK_WAKEUP delivered for this port can therefore run concurrently with
the purge: tipc_sk_rcv() looks the socket up and takes a reference before
tipc_sk_remove() unhashes it, is then delayed past release_sock() so
sock_owned_by_user() is false, its spin_trylock_bh() succeeds, and it
list_del()s and kfree()s a struct tipc_dest that the closing task is
walking at the same time.  Both paths free entries of the same list.

__tipc_shutdown() does not close this window: it waits on
!tsk->cong_link_cnt but ignores the return value of tipc_wait_for_cond(),
which returns early on timeout, on a pending signal, or on sk_err, so the
close can proceed with cong_links still populated.

  BUG: KASAN: slab-use-after-free in __list_del_entry_valid_or_report+0x1ce/0x280
  Read of size 8 at addr ffff888105ddda88 by task poc_cong_race/7856
   __list_del_entry_valid_or_report+0x1ce/0x280
   tipc_dest_list_purge+0xad/0x240
   tipc_release+0x9a5/0x1340
  Allocated by task 7856:
   tipc_dest_push+0x11c/0x2f0
   __tipc_sendmsg+0x1443/0x17a0
  Freed by task 7851:
   tipc_dest_del+0x1ce/0x280
   tipc_sk_filter_rcv+0x1da7/0x2e70
   tipc_sk_rcv+0xdaa/0x1a80
   tipc_udp_recv+0x42a/0x7e0
  Oops: general protection fault ... Kernel panic - not syncing: Fatal exception

Move the purge above release_sock() so it runs under the socket lock, the
same discipline commit 844cf763fba6 ("tipc: make macro tipc_wait_for_cond()
smp safe") established for the wait condition.  A concurrent tipc_sk_rcv()
then either backlogs the wakeup because the socket is owned, or processes
it against an already empty list, and no new delivery can arrive because
tipc_sk_remove() has already unhashed the socket.

Fixes: 365ad353c256 ("tipc: reduce risk of user starvation during link congestion")
Cc: stable@kernel.org
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Signed-off-by: Jun Yang <junvyyang@tencent.com>
---
 net/tipc/socket.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index d5d70eb230b5..9b45b16d0a31 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -647,11 +647,17 @@ static int tipc_release(struct socket *sock)
 	sk_stop_timer(sk, &sk->sk_timer);
 	tipc_sk_remove(tsk);
 
+	/* Purge under the socket lock: a straggler SOCK_WAKEUP that looked the
+	 * socket up before tipc_sk_remove() can still reach tipc_dest_del() on
+	 * this list, and it only holds the socket spinlock.  Purging after
+	 * release_sock() would race that list_del()/kfree().
+	 */
+	tipc_dest_list_purge(&tsk->cong_links);
+	tsk->cong_link_cnt = 0;
+
 	sock_orphan(sk);
 	/* Reject any messages that accumulated in backlog queue */
 	release_sock(sk);
-	tipc_dest_list_purge(&tsk->cong_links);
-	tsk->cong_link_cnt = 0;
 	call_rcu(&tsk->rcu, tipc_sk_callback);
 	sock->sk = NULL;
 
-- 
2.55.0