Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpf: Fix use-after-free of sockmap #8686

Open
wants to merge 3 commits into
base: bpf-next_base
Choose a base branch
from
Open

bpf: Fix use-after-free of sockmap #8686

wants to merge 3 commits into from
+91 −4

Conversation

kernel-patches-daemon-bpf[bot]
Copy link

Pull request for series with
subject: bpf: Fix use-after-free of sockmap
version: 3
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 812f770
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: b02f072
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f3f8649
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: ae0a457
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f4edc66
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 6ca2162
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: a259804
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 79db658
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: e16e64f
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 51d6504
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 307ef66
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 9aa8fe2
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 9aa8fe2
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

@mrpre
bpf, sockmap: avoid using sk_socket after free when sending
77d32a7 
The sk->sk_socket is not locked or referenced, 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
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
'''

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.
'''
void sock_map_close()
{
    ...
    if (likely(psock)) {
    ...
    psock = sk_psock_get(sk);
    if (unlikely(!psock))
        goto no_psock; <=== Control usually jumps here via goto
        ...
        cancel_delayed_work_sync(&psock->work); <=== not executed
        sk_psock_put(sk, psock);
        ...
}
'''

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
...
'''

Reported-by: Michal Luczaj <[email protected]>
Fixes: 799aa7f ("skmsg: Avoid lock_sock() in sk_psock_backlog()")
Signed-off-by: Jiayuan Chen <[email protected]>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 9aa8fe2
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=944583
version: 3

mrpre added 2 commits March 24, 2025 11:25
@mrpre
bpf, sockmap: avoid using sk_socket after free when reading
f5ac942 
There are potential concurrency issues, as shown below.
'''
CPU0                               CPU1
sk_psock_verdict_data_ready:
  socket *sock = sk->sk_socket
  if (!sock) return
                                   close(fd):
                                     ...
                                     ops->release()
  if (!sock->ops) return
                                     sock->ops = NULL
                                     rcu_call(sock)
                                     free(sock)
  READ_ONCE(sock->ops)
  ^
  use 'sock' after free
'''

RCU is not applicable to Unix sockets read path, because the Unix socket
implementation itself assumes it's always in process context and heavily
uses mutex_lock, so, we can't call read_skb within rcu lock.

Incrementing the psock reference count would not help either, since
sock_map_close() does not wait for data_ready() to complete its execution.

While we don't utilize sk_socket here, implementing read_skb at the sock
layer instead of socket layer might be architecturally preferable ?
However, deferring this optimization as current fix adequately addresses
the immediate issue.

Fixes: c638291 ("af_unix: Implement ->psock_update_sk_prot()")
Reported-by: [email protected]
Closes: https://lore.kernel.org/bpf/[email protected]/
Signed-off-by: Jiayuan Chen <[email protected]>
@mrpre
selftests/bpf: Add edge case tests for sockmap
3c64a8d 
Add edge case tests for sockmap.

Signed-off-by: Jiayuan Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bpf-next new V3-ci-pass V3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@mrpre