From nobody Mon Dec 15 21:49:50 2025 Received: from out-183.mta0.migadu.com (out-183.mta0.migadu.com [91.218.175.183]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B6CE31632DF for ; Fri, 16 May 2025 14:17:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.183 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1747405074; cv=none; b=FUmsMrKo6yeVFyMB9XEeOF7R6VMbYbW39vzTEhEPlgJ5rVCitvSPRqDZS8qmBCNuIJ79oN7bH/2JZ/RvNPIT5U7moCOsH6dq4kKxDPEAYgtwrf+LidTo26SGU6Lb8hQZI/WFCt6f7CFGv7mfKXQOTgZFJduOIiNS2uU8c+CSWy4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1747405074; c=relaxed/simple; bh=di7VcqmsOR/ksmcCT55gbdF3amMK+ETAgYq2FlW2QA8=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=jJw0W+Qn03M1Hj5tFAcyF8x8xh2tcj4va464YfJpNvYcarIXGOuoH8iaP/eQETSnD+aoFGOTyrkd9CMcfaZinVBTUSs39gI4uKfBF7PB6QXbjr61eP/E6PwEDeLjsMhx9qqPsdBcW0C88iyt7H1ZEVBK3Vag/h96ajkvJ2tifmk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=XtXRnzWg; arc=none smtp.client-ip=91.218.175.183 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="XtXRnzWg" X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1747405060; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=2WuqP8hVhA8NoR8vkcRmgIaGt57MUyF4WZKw8XNlY3A=; b=XtXRnzWg1XOU15tFoqBgtCFbrOgBRiT7vGUxMP8epA3uZ4lF0it8pMe1uNaV+HyNBzFFrn sGv4+MERAu4wsBhIr4sEv6MvLdRkj7lshSJoDgWgxbrajRIK8lbexfkw6+wwTag7erT9E1 qFouPCUZPmw/ouaJeyusQBLxSkzKA0A= From: Jiayuan Chen To: bpf@vger.kernel.org Cc: Jiayuan Chen , Michal Luczaj , John Fastabend , Jakub Sitnicki , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , Thadeu Lima de Souza Cascardo , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH bpf-next v6] bpf, sockmap: avoid using sk_socket after free when sending Date: Fri, 16 May 2025 22:17:12 +0800 Message-ID: <20250516141713.291150-1-jiayuan.chen@linux.dev> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Migadu-Flow: FLOW_OUT Content-Type: text/plain; charset="utf-8" The sk->sk_socket is not locked or referenced in backlog thread, and during the call to skb_send_sock(), there is a race condition with the release of sk_socket. All types of sockets(tcp/udp/unix/vsock) will be affected. Race conditions: ''' CPU0 CPU1 backlog::skb_send_sock sendmsg_unlocked sock_sendmsg sock_sendmsg_nosec close(fd): ... ops->release() -> sock_map_close() sk_socket->ops =3D NULL free(socket) sock->ops->sendmsg ^ panic here ''' The ref of psock become 0 after sock_map_close() executed. ''' void sock_map_close() { ... if (likely(psock)) { ... // !! here we remove psock and the ref of psock become 0 sock_map_remove_links(sk, psock) psock =3D sk_psock_get(sk); if (unlikely(!psock)) goto no_psock; <=3D=3D=3D Control jumps here via goto ... cancel_delayed_work_sync(&psock->work); <=3D=3D=3D not executed sk_psock_put(sk, psock); ... } ''' Based on the fact that we already wait for the workqueue to finish in sock_map_close() if psock is held, we simply increase the psock reference count to avoid race conditions. With this patch, if the backlog thread is running, sock_map_close() will wait for the backlog thread to complete and cancel all pending work. If no backlog running, any pending work that hasn't started by then will fail when invoked by sk_psock_get(), as the psock reference count have been zeroed, and sk_psock_drop() will cancel all jobs via cancel_delayed_work_sync(). In summary, we require synchronization to coordinate the backlog thread and close() thread. The panic I catched: ''' Workqueue: events sk_psock_backlog RIP: 0010:sock_sendmsg+0x21d/0x440 RAX: 0000000000000000 RBX: ffffc9000521fad8 RCX: 0000000000000001 ... Call Trace: ? die_addr+0x40/0xa0 ? exc_general_protection+0x14c/0x230 ? asm_exc_general_protection+0x26/0x30 ? sock_sendmsg+0x21d/0x440 ? sock_sendmsg+0x3e0/0x440 ? __pfx_sock_sendmsg+0x10/0x10 __skb_send_sock+0x543/0xb70 sk_psock_backlog+0x247/0xb80 ... ''' Reported-by: Michal Luczaj Fixes: 4b4647add7d3 ("sock_map: avoid race between sock_map_close and sk_ps= ock_put") Signed-off-by: Jiayuan Chen Reviewed-by: John Fastabend --- V5 -> V6: Use correct "Fixes" tag. V4 -> V5: This patch is extracted from my previous v4 patchset that contained multiple fixes, and it remains unchanged. Since this fix is relatively simple and easy to review, we want to separate it from other fixes to avoid any potential interference. --- net/core/skmsg.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/net/core/skmsg.c b/net/core/skmsg.c index 276934673066..34c51eb1a14f 100644 --- a/net/core/skmsg.c +++ b/net/core/skmsg.c @@ -656,6 +656,13 @@ static void sk_psock_backlog(struct work_struct *work) bool ingress; int ret; =20 + /* Increment the psock refcnt to synchronize with close(fd) path in + * sock_map_close(), ensuring we wait for backlog thread completion + * before sk_socket freed. If refcnt increment fails, it indicates + * sock_map_close() completed with sk_socket potentially already freed. + */ + if (!sk_psock_get(psock->sk)) + return; mutex_lock(&psock->work_mutex); while ((skb =3D skb_peek(&psock->ingress_skb))) { len =3D skb->len; @@ -708,6 +715,7 @@ static void sk_psock_backlog(struct work_struct *work) } end: mutex_unlock(&psock->work_mutex); + sk_psock_put(psock->sk, psock); } =20 struct sk_psock *sk_psock_init(struct sock *sk, int node) --=20 2.47.1