Skip to content

Commit a3ebd58

Browse files
committed
bpf, sockmap: Avoid using sk_socket after free when sending
JIRA: https://issues.redhat.com/browse/RHEL-84579 commit 8259eb0 Author: Jiayuan Chen <jiayuan.chen@linux.dev> Date: Fri May 16 22:17:12 2025 +0800 bpf, sockmap: Avoid using sk_socket after free when sending 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 = 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 = sk_psock_get(sk); if (unlikely(!psock)) goto no_psock; <=== Control jumps here via goto ... cancel_delayed_work_sync(&psock->work); <=== 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: <TASK> ? 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 ... ''' Fixes: 4b4647a ("sock_map: avoid race between sock_map_close and sk_psock_put") Reported-by: Michal Luczaj <mhal@rbox.co> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Reviewed-by: John Fastabend <john.fastabend@gmail.com> Link: https://lore.kernel.org/r/20250516141713.291150-1-jiayuan.chen@linux.dev Signed-off-by: Felix Maurer <fmaurer@redhat.com>
1 parent ea453b2 commit a3ebd58

File tree

1 file changed

+8
-0
lines changed

1 file changed

+8
-0
lines changed

net/core/skmsg.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,13 @@ static void sk_psock_backlog(struct work_struct *work)
647647
u32 len, off;
648648
int ret;
649649

650+
/* Increment the psock refcnt to synchronize with close(fd) path in
651+
* sock_map_close(), ensuring we wait for backlog thread completion
652+
* before sk_socket freed. If refcnt increment fails, it indicates
653+
* sock_map_close() completed with sk_socket potentially already freed.
654+
*/
655+
if (!sk_psock_get(psock->sk))
656+
return;
650657
mutex_lock(&psock->work_mutex);
651658
if (unlikely(state->skb)) {
652659
spin_lock_bh(&psock->ingress_lock);
@@ -697,6 +704,7 @@ static void sk_psock_backlog(struct work_struct *work)
697704
}
698705
end:
699706
mutex_unlock(&psock->work_mutex);
707+
sk_psock_put(psock->sk, psock);
700708
}
701709

702710
struct sk_psock *sk_psock_init(struct sock *sk, int node)

0 commit comments

Comments
 (0)