* [PATCH bpf-next v1 0/3] bpf: Fix use-after-free of sockmap
@ 2025-02-26 13:22 Jiayuan Chen
2025-02-26 13:22 ` [PATCH bpf-next v1 1/3] bpf, sockmap: avoid using sk_socket after free Jiayuan Chen
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Jiayuan Chen @ 2025-02-26 13:22 UTC (permalink / raw)
To: cong.wang, john.fastabend, jakub
Cc: davem, edumazet, kuba, pabeni, horms, andrii, eddyz87, mykolal,
ast, daniel, martin.lau, song, yonghong.song, kpsingh, sdf,
haoluo, jolsa, shuah, mhal, jiayuan.chen, sgarzare, netdev, bpf,
linux-kernel, linux-kselftest, mrpre
1. Issue
Syzkaller reported this issue [1].
2. Reproduce
We can reproduce this issue by using the test_sockmap_with_close_on_write()
test I provided in selftest, also you need to apply the following patch to
ensure 100% reproducibility (sleep after checking sock):
'''
static void sk_psock_verdict_data_ready(struct sock *sk)
{
.......
if (unlikely(!sock))
return;
+ if (!strcmp("test_progs", current->comm)) {
+ printk("sleep 2s to wait socket freed\n");
+ mdelay(2000);
+ printk("sleep end\n");
+ }
ops = READ_ONCE(sock->ops);
if (!ops || !ops->read_skb)
return;
}
'''
Then running './test_progs -v sockmap_basic', and if the kernel has KASAN
enabled [2], you will see the following warning:
'''
BUG: KASAN: slab-use-after-free in sk_psock_verdict_data_ready+0x29b/0x2d0
Read of size 8 at addr ffff88813a777020 by task test_progs/47055
Tainted: [O]=OOT_MODULE
Call Trace:
<TASK>
dump_stack_lvl+0x53/0x70
print_address_description.constprop.0+0x30/0x420
? sk_psock_verdict_data_ready+0x29b/0x2d0
print_report+0xb7/0x270
? sk_psock_verdict_data_ready+0x29b/0x2d0
? kasan_addr_to_slab+0xd/0xa0
? sk_psock_verdict_data_ready+0x29b/0x2d0
kasan_report+0xca/0x100
? sk_psock_verdict_data_ready+0x29b/0x2d0
sk_psock_verdict_data_ready+0x29b/0x2d0
unix_stream_sendmsg+0x4a6/0xa40
? __pfx_unix_stream_sendmsg+0x10/0x10
? fdget+0x2c1/0x3a0
__sys_sendto+0x39c/0x410
'''
3. Reason
'''
CPU0 CPU1
unix_stream_sendmsg(sk):
other = unix_peer(sk)
other->sk_data_ready(other):
socket *sock = sk->sk_socket
if (unlikely(!sock))
return;
close(other):
...
other->close()
free(socket)
READ_ONCE(sock->ops)
^
use 'sock' after free
'''
For TCP, UDP, or other protocols, we have already performed
rcu_read_lock() when the network stack receives packets in ip_input.c:
'''
ip_local_deliver_finish():
rcu_read_lock()
ip_protocol_deliver_rcu()
xxx_rcv
rcu_read_unlock()
'''
However, for Unix sockets, sk_data_ready is called directly from the
process context without rcu_read_lock() protection.
4. Solution
Based on the fact that the 'struct socket' is released using call_rcu(),
We add rcu_read_{un}lock() at the entrance and exit of our sk_data_ready.
It will not increase performance overhead, at least for TCP and UDP, they
are already in a relatively large critical section.
Of course, we can also add a custom callback for Unix sockets and call
rcu_read_lock() before calling _verdict_data_ready like this:
'''
if (sk_is_unix(sk))
sk->sk_data_ready = sk_psock_verdict_data_ready_rcu;
else
sk->sk_data_ready = sk_psock_verdict_data_ready;
sk_psock_verdict_data_ready_rcu():
rcu_read_lock()
sk_psock_verdict_data_ready()
rcu_read_unlock()
'''
However, this will cause too many branches, and it's not suitable to
distinguish network protocols in skmsg.c.
[1] https://syzkaller.appspot.com/bug?extid=dd90a702f518e0eac072
[2] https://syzkaller.appspot.com/text?tag=KernelConfig&x=1362a5aee630ff34
Jiayuan Chen (3):
bpf, sockmap: avoid using sk_socket after free
selftests/bpf: Add socketpair to create_pair to support unix socket
selftests/bpf: Add edge case tests for sockmap
net/core/skmsg.c | 18 ++++--
.../selftests/bpf/prog_tests/socket_helpers.h | 13 ++++-
.../selftests/bpf/prog_tests/sockmap_basic.c | 57 +++++++++++++++++++
3 files changed, 82 insertions(+), 6 deletions(-)
--
2.47.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH bpf-next v1 1/3] bpf, sockmap: avoid using sk_socket after free
2025-02-26 13:22 [PATCH bpf-next v1 0/3] bpf: Fix use-after-free of sockmap Jiayuan Chen
@ 2025-02-26 13:22 ` Jiayuan Chen
2025-02-27 19:45 ` Cong Wang
2025-02-27 23:04 ` Martin KaFai Lau
2025-02-26 13:22 ` [PATCH bpf-next v1 2/3] selftests/bpf: Add socketpair to create_pair to support unix socket Jiayuan Chen
2025-02-26 13:22 ` [PATCH bpf-next v1 3/3] selftests/bpf: Add edge case tests for sockmap Jiayuan Chen
2 siblings, 2 replies; 14+ messages in thread
From: Jiayuan Chen @ 2025-02-26 13:22 UTC (permalink / raw)
To: cong.wang, john.fastabend, jakub
Cc: davem, edumazet, kuba, pabeni, horms, andrii, eddyz87, mykolal,
ast, daniel, martin.lau, song, yonghong.song, kpsingh, sdf,
haoluo, jolsa, shuah, mhal, jiayuan.chen, sgarzare, netdev, bpf,
linux-kernel, linux-kselftest, mrpre, syzbot+dd90a702f518e0eac072
Use RCU lock to protect sk_socket, preventing concurrent close and release
by another thread.
Because TCP/UDP are already within a relatively large critical section:
'''
ip_local_deliver_finish
rcu_read_lock
ip_protocol_deliver_rcu
tcp_rcv/udp_rcv
rcu_read_unlock
'''
Adding rcu_read_{un}lock() at the entrance and exit of sk_data_ready
will not increase performance overhead.
Reported-by: syzbot+dd90a702f518e0eac072@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/bpf/6734c033.050a0220.2a2fcc.0015.GAE@google.com/
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
net/core/skmsg.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index 0ddc4c718833..1b71ae1d1bf5 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -1222,27 +1222,35 @@ static int sk_psock_verdict_recv(struct sock *sk, struct sk_buff *skb)
static void sk_psock_verdict_data_ready(struct sock *sk)
{
- struct socket *sock = sk->sk_socket;
+ struct socket *sock;
const struct proto_ops *ops;
int copied;
trace_sk_data_ready(sk);
+ /* We need RCU to prevent the sk_socket from being released.
+ * Especially for Unix sockets, we are currently in the process
+ * context and do not have RCU protection.
+ */
+ rcu_read_lock();
+ sock = sk->sk_socket;
if (unlikely(!sock))
- return;
+ goto unlock;
+
ops = READ_ONCE(sock->ops);
if (!ops || !ops->read_skb)
- return;
+ goto unlock;
+
copied = ops->read_skb(sk, sk_psock_verdict_recv);
if (copied >= 0) {
struct sk_psock *psock;
- rcu_read_lock();
psock = sk_psock(sk);
if (psock)
sk_psock_data_ready(sk, psock);
- rcu_read_unlock();
}
+unlock:
+ rcu_read_unlock();
}
void sk_psock_start_verdict(struct sock *sk, struct sk_psock *psock)
--
2.47.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH bpf-next v1 2/3] selftests/bpf: Add socketpair to create_pair to support unix socket
2025-02-26 13:22 [PATCH bpf-next v1 0/3] bpf: Fix use-after-free of sockmap Jiayuan Chen
2025-02-26 13:22 ` [PATCH bpf-next v1 1/3] bpf, sockmap: avoid using sk_socket after free Jiayuan Chen
@ 2025-02-26 13:22 ` Jiayuan Chen
2025-02-27 19:52 ` Cong Wang
2025-02-26 13:22 ` [PATCH bpf-next v1 3/3] selftests/bpf: Add edge case tests for sockmap Jiayuan Chen
2 siblings, 1 reply; 14+ messages in thread
From: Jiayuan Chen @ 2025-02-26 13:22 UTC (permalink / raw)
To: cong.wang, john.fastabend, jakub
Cc: davem, edumazet, kuba, pabeni, horms, andrii, eddyz87, mykolal,
ast, daniel, martin.lau, song, yonghong.song, kpsingh, sdf,
haoluo, jolsa, shuah, mhal, jiayuan.chen, sgarzare, netdev, bpf,
linux-kernel, linux-kselftest, mrpre
Current wrapper function create_pair() is used to create a pair of
connected links and returns two fds, but it does not support unix sockets.
Here we introduce socketpair() into create_pair(), which supports creating
a pair of unix sockets, since the semantics of the two are the same.
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
.../selftests/bpf/prog_tests/socket_helpers.h | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/socket_helpers.h b/tools/testing/selftests/bpf/prog_tests/socket_helpers.h
index 1bdfb79ef009..a805143dd84f 100644
--- a/tools/testing/selftests/bpf/prog_tests/socket_helpers.h
+++ b/tools/testing/selftests/bpf/prog_tests/socket_helpers.h
@@ -313,11 +313,22 @@ static inline int recv_timeout(int fd, void *buf, size_t len, int flags,
static inline int create_pair(int family, int sotype, int *p0, int *p1)
{
- __close_fd int s, c = -1, p = -1;
+ __close_fd int s = -1, c = -1, p = -1;
struct sockaddr_storage addr;
socklen_t len = sizeof(addr);
int err;
+ if (family == AF_UNIX) {
+ int fds[2];
+
+ err = socketpair(family, sotype, 0, fds);
+ if (!err) {
+ *p0 = fds[0];
+ *p1 = fds[1];
+ }
+ return err;
+ }
+
s = socket_loopback(family, sotype);
if (s < 0)
return s;
--
2.47.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH bpf-next v1 3/3] selftests/bpf: Add edge case tests for sockmap
2025-02-26 13:22 [PATCH bpf-next v1 0/3] bpf: Fix use-after-free of sockmap Jiayuan Chen
2025-02-26 13:22 ` [PATCH bpf-next v1 1/3] bpf, sockmap: avoid using sk_socket after free Jiayuan Chen
2025-02-26 13:22 ` [PATCH bpf-next v1 2/3] selftests/bpf: Add socketpair to create_pair to support unix socket Jiayuan Chen
@ 2025-02-26 13:22 ` Jiayuan Chen
2025-02-27 19:53 ` Cong Wang
2025-02-27 22:18 ` John Fastabend
2 siblings, 2 replies; 14+ messages in thread
From: Jiayuan Chen @ 2025-02-26 13:22 UTC (permalink / raw)
To: cong.wang, john.fastabend, jakub
Cc: davem, edumazet, kuba, pabeni, horms, andrii, eddyz87, mykolal,
ast, daniel, martin.lau, song, yonghong.song, kpsingh, sdf,
haoluo, jolsa, shuah, mhal, jiayuan.chen, sgarzare, netdev, bpf,
linux-kernel, linux-kselftest, mrpre
Add edge case tests for sockmap.
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
.../selftests/bpf/prog_tests/sockmap_basic.c | 57 +++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 1e3e4392dcca..e36b5c78fc98 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -1042,6 +1042,61 @@ static void test_sockmap_vsock_unconnected(void)
xclose(map);
}
+void *close_thread(void *arg)
+{
+ int fd = *(int *)arg;
+
+ sleep(1);
+ close(fd);
+ return NULL;
+}
+
+void test_sockmap_with_close_on_write(int family, int sotype)
+{
+ struct test_sockmap_pass_prog *skel;
+ int err, map, verdict, sent;
+ pthread_t tid;
+ int zero = 0;
+ int c = -1, p = -1;
+
+ skel = test_sockmap_pass_prog__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "open_and_load"))
+ return;
+
+ verdict = bpf_program__fd(skel->progs.prog_skb_verdict);
+ map = bpf_map__fd(skel->maps.sock_map_rx);
+
+ err = bpf_prog_attach(verdict, map, BPF_SK_SKB_STREAM_VERDICT, 0);
+ if (!ASSERT_OK(err, "bpf_prog_attach"))
+ goto out;
+
+ err = create_pair(family, sotype, &c, &p);
+ if (!ASSERT_OK(err, "create_pair"))
+ goto out;
+
+ err = bpf_map_update_elem(map, &zero, &p, BPF_ANY);
+ if (!ASSERT_OK(err, "bpf_map_update_elem"))
+ goto out;
+
+ err = pthread_create(&tid, 0, close_thread, &p);
+ if (!ASSERT_OK(err, "pthread_create"))
+ goto out;
+
+ sent = xsend(c, "a", 1, 0);
+ if (!ASSERT_EQ(sent, 1, "xsend"))
+ goto out;
+
+ pthread_join(tid, NULL);
+ /* p was closed in thread */
+ p = -1;
+out:
+ if (c > 0)
+ close(c);
+ if (p > 0)
+ close(p);
+ test_sockmap_pass_prog__destroy(skel);
+}
+
void test_sockmap_basic(void)
{
if (test__start_subtest("sockmap create_update_free"))
@@ -1108,4 +1163,6 @@ void test_sockmap_basic(void)
test_sockmap_skb_verdict_vsock_poll();
if (test__start_subtest("sockmap vsock unconnected"))
test_sockmap_vsock_unconnected();
+ if (test__start_subtest("sockmap with unix and close"))
+ test_sockmap_with_close_on_write(AF_UNIX, SOCK_STREAM);
}
--
2.47.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH bpf-next v1 1/3] bpf, sockmap: avoid using sk_socket after free
2025-02-26 13:22 ` [PATCH bpf-next v1 1/3] bpf, sockmap: avoid using sk_socket after free Jiayuan Chen
@ 2025-02-27 19:45 ` Cong Wang
2025-02-27 20:57 ` John Fastabend
2025-02-27 23:04 ` Martin KaFai Lau
1 sibling, 1 reply; 14+ messages in thread
From: Cong Wang @ 2025-02-27 19:45 UTC (permalink / raw)
To: Jiayuan Chen
Cc: cong.wang, john.fastabend, jakub, davem, edumazet, kuba, pabeni,
horms, andrii, eddyz87, mykolal, ast, daniel, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, shuah, mhal, sgarzare,
netdev, bpf, linux-kernel, linux-kselftest, mrpre,
syzbot+dd90a702f518e0eac072
On Wed, Feb 26, 2025 at 09:22:40PM +0800, Jiayuan Chen wrote:
> Use RCU lock to protect sk_socket, preventing concurrent close and release
> by another thread.
>
> Because TCP/UDP are already within a relatively large critical section:
> '''
> ip_local_deliver_finish
> rcu_read_lock
> ip_protocol_deliver_rcu
> tcp_rcv/udp_rcv
> rcu_read_unlock
> '''
>
> Adding rcu_read_{un}lock() at the entrance and exit of sk_data_ready
> will not increase performance overhead.
>
> Reported-by: syzbot+dd90a702f518e0eac072@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/bpf/6734c033.050a0220.2a2fcc.0015.GAE@google.com/
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
sock_def_readable() already acquires RCU read lock anyway.
Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf-next v1 2/3] selftests/bpf: Add socketpair to create_pair to support unix socket
2025-02-26 13:22 ` [PATCH bpf-next v1 2/3] selftests/bpf: Add socketpair to create_pair to support unix socket Jiayuan Chen
@ 2025-02-27 19:52 ` Cong Wang
2025-02-27 22:21 ` John Fastabend
2025-02-28 4:51 ` Jiayuan Chen
0 siblings, 2 replies; 14+ messages in thread
From: Cong Wang @ 2025-02-27 19:52 UTC (permalink / raw)
To: Jiayuan Chen
Cc: cong.wang, john.fastabend, jakub, davem, edumazet, kuba, pabeni,
horms, andrii, eddyz87, mykolal, ast, daniel, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, shuah, mhal, sgarzare,
netdev, bpf, linux-kernel, linux-kselftest, mrpre
On Wed, Feb 26, 2025 at 09:22:41PM +0800, Jiayuan Chen wrote:
> Current wrapper function create_pair() is used to create a pair of
> connected links and returns two fds, but it does not support unix sockets.
>
> Here we introduce socketpair() into create_pair(), which supports creating
> a pair of unix sockets, since the semantics of the two are the same.
Since it is only for UDS and only has effectively 1 line of code, how
about just calling socketpair(AF_UNIX) in your patch 3/3?
Thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf-next v1 3/3] selftests/bpf: Add edge case tests for sockmap
2025-02-26 13:22 ` [PATCH bpf-next v1 3/3] selftests/bpf: Add edge case tests for sockmap Jiayuan Chen
@ 2025-02-27 19:53 ` Cong Wang
2025-02-27 22:18 ` John Fastabend
1 sibling, 0 replies; 14+ messages in thread
From: Cong Wang @ 2025-02-27 19:53 UTC (permalink / raw)
To: Jiayuan Chen
Cc: cong.wang, john.fastabend, jakub, davem, edumazet, kuba, pabeni,
horms, andrii, eddyz87, mykolal, ast, daniel, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, shuah, mhal, sgarzare,
netdev, bpf, linux-kernel, linux-kselftest, mrpre
On Wed, Feb 26, 2025 at 09:22:42PM +0800, Jiayuan Chen wrote:
> Add edge case tests for sockmap.
>
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
I always love to see fixes with test cases.
Thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf-next v1 1/3] bpf, sockmap: avoid using sk_socket after free
2025-02-27 19:45 ` Cong Wang
@ 2025-02-27 20:57 ` John Fastabend
0 siblings, 0 replies; 14+ messages in thread
From: John Fastabend @ 2025-02-27 20:57 UTC (permalink / raw)
To: Cong Wang
Cc: Jiayuan Chen, cong.wang, jakub, davem, edumazet, kuba, pabeni,
horms, andrii, eddyz87, mykolal, ast, daniel, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, shuah, mhal, sgarzare,
netdev, bpf, linux-kernel, linux-kselftest, mrpre,
syzbot+dd90a702f518e0eac072
On 2025-02-27 11:45:53, Cong Wang wrote:
> On Wed, Feb 26, 2025 at 09:22:40PM +0800, Jiayuan Chen wrote:
> > Use RCU lock to protect sk_socket, preventing concurrent close and release
> > by another thread.
> >
> > Because TCP/UDP are already within a relatively large critical section:
> > '''
> > ip_local_deliver_finish
> > rcu_read_lock
> > ip_protocol_deliver_rcu
> > tcp_rcv/udp_rcv
> > rcu_read_unlock
> > '''
> >
> > Adding rcu_read_{un}lock() at the entrance and exit of sk_data_ready
> > will not increase performance overhead.
> >
> > Reported-by: syzbot+dd90a702f518e0eac072@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/bpf/6734c033.050a0220.2a2fcc.0015.GAE@google.com/
> > Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>
> sock_def_readable() already acquires RCU read lock anyway.
>
> Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks.
Reviewed-by: John Fastabend <john.fastabend@gmail.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf-next v1 3/3] selftests/bpf: Add edge case tests for sockmap
2025-02-26 13:22 ` [PATCH bpf-next v1 3/3] selftests/bpf: Add edge case tests for sockmap Jiayuan Chen
2025-02-27 19:53 ` Cong Wang
@ 2025-02-27 22:18 ` John Fastabend
1 sibling, 0 replies; 14+ messages in thread
From: John Fastabend @ 2025-02-27 22:18 UTC (permalink / raw)
To: Jiayuan Chen
Cc: cong.wang, jakub, davem, edumazet, kuba, pabeni, horms, andrii,
eddyz87, mykolal, ast, daniel, martin.lau, song, yonghong.song,
kpsingh, sdf, haoluo, jolsa, shuah, mhal, sgarzare, netdev, bpf,
linux-kernel, linux-kselftest, mrpre
On 2025-02-26 21:22:42, Jiayuan Chen wrote:
> Add edge case tests for sockmap.
>
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
Reviewed-by: John Fastabend <john.fastabend@gmail.com>
> .../selftests/bpf/prog_tests/sockmap_basic.c | 57> +
> void test_sockmap_basic(void)
> {
> if (test__start_subtest("sockmap create_update_free"))
> @@ -1108,4 +1163,6 @@ void test_sockmap_basic(void)
> test_sockmap_skb_verdict_vsock_poll();
> if (test__start_subtest("sockmap vsock unconnected"))
> test_sockmap_vsock_unconnected();
> + if (test__start_subtest("sockmap with unix and close"))
> + test_sockmap_with_close_on_write(AF_UNIX, SOCK_STREAM);
Might also be worth running these with other types? No bug was
fixed there but still testing close while writing is useful
for testing RCU paths in TCP/UDP.
Fine as a follow up patch IMO.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf-next v1 2/3] selftests/bpf: Add socketpair to create_pair to support unix socket
2025-02-27 19:52 ` Cong Wang
@ 2025-02-27 22:21 ` John Fastabend
2025-02-28 4:51 ` Jiayuan Chen
2025-02-28 4:51 ` Jiayuan Chen
1 sibling, 1 reply; 14+ messages in thread
From: John Fastabend @ 2025-02-27 22:21 UTC (permalink / raw)
To: Cong Wang
Cc: Jiayuan Chen, cong.wang, jakub, davem, edumazet, kuba, pabeni,
horms, andrii, eddyz87, mykolal, ast, daniel, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, shuah, mhal, sgarzare,
netdev, bpf, linux-kernel, linux-kselftest, mrpre
On 2025-02-27 11:52:04, Cong Wang wrote:
> On Wed, Feb 26, 2025 at 09:22:41PM +0800, Jiayuan Chen wrote:
> > Current wrapper function create_pair() is used to create a pair of
> > connected links and returns two fds, but it does not support unix sockets.
> >
> > Here we introduce socketpair() into create_pair(), which supports creating
> > a pair of unix sockets, since the semantics of the two are the same.
>
> Since it is only for UDS and only has effectively 1 line of code, how
> about just calling socketpair(AF_UNIX) in your patch 3/3?
If we run that test with more than AF_UNIX it might be best as is. I
think there might be some value testing that flow on TCP/UDP even if
its not related to the bug.
Thanks,
John
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf-next v1 1/3] bpf, sockmap: avoid using sk_socket after free
2025-02-26 13:22 ` [PATCH bpf-next v1 1/3] bpf, sockmap: avoid using sk_socket after free Jiayuan Chen
2025-02-27 19:45 ` Cong Wang
@ 2025-02-27 23:04 ` Martin KaFai Lau
2025-02-28 4:49 ` Jiayuan Chen
1 sibling, 1 reply; 14+ messages in thread
From: Martin KaFai Lau @ 2025-02-27 23:04 UTC (permalink / raw)
To: Jiayuan Chen
Cc: cong.wang, john.fastabend, jakub, davem, edumazet, kuba, pabeni,
horms, andrii, eddyz87, mykolal, ast, daniel, song, yonghong.song,
kpsingh, sdf, haoluo, jolsa, shuah, mhal, sgarzare, netdev, bpf,
linux-kernel, linux-kselftest, mrpre, syzbot+dd90a702f518e0eac072
On 2/26/25 5:22 AM, Jiayuan Chen wrote:
> Use RCU lock to protect sk_socket, preventing concurrent close and release
> by another thread.
>
> Because TCP/UDP are already within a relatively large critical section:
> '''
> ip_local_deliver_finish
> rcu_read_lock
> ip_protocol_deliver_rcu
> tcp_rcv/udp_rcv
> rcu_read_unlock
> '''
>
> Adding rcu_read_{un}lock() at the entrance and exit of sk_data_ready
> will not increase performance overhead.
Can it use a Fixes tag?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf-next v1 1/3] bpf, sockmap: avoid using sk_socket after free
2025-02-27 23:04 ` Martin KaFai Lau
@ 2025-02-28 4:49 ` Jiayuan Chen
0 siblings, 0 replies; 14+ messages in thread
From: Jiayuan Chen @ 2025-02-28 4:49 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: cong.wang, john.fastabend, jakub, davem, edumazet, kuba, pabeni,
horms, andrii, eddyz87, mykolal, ast, daniel, song, yonghong.song,
kpsingh, sdf, haoluo, jolsa, shuah, mhal, sgarzare, netdev, bpf,
linux-kernel, linux-kselftest, mrpre, syzbot+dd90a702f518e0eac072
On Thu, Feb 27, 2025 at 03:04:26PM -0800, Martin KaFai Lau wrote:
> On 2/26/25 5:22 AM, Jiayuan Chen wrote:
> > Use RCU lock to protect sk_socket, preventing concurrent close and release
> > by another thread.
> >
> > Because TCP/UDP are already within a relatively large critical section:
> > '''
> > ip_local_deliver_finish
> > rcu_read_lock
> > ip_protocol_deliver_rcu
> > tcp_rcv/udp_rcv
> > rcu_read_unlock
> > '''
> >
> > Adding rcu_read_{un}lock() at the entrance and exit of sk_data_ready
> > will not increase performance overhead.
>
> Can it use a Fixes tag?
Thanks, Martin.
It seems that this issue has existed since sockmap supported unix.
I'll find the corresponding commit as the Fixes tag.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf-next v1 2/3] selftests/bpf: Add socketpair to create_pair to support unix socket
2025-02-27 19:52 ` Cong Wang
2025-02-27 22:21 ` John Fastabend
@ 2025-02-28 4:51 ` Jiayuan Chen
1 sibling, 0 replies; 14+ messages in thread
From: Jiayuan Chen @ 2025-02-28 4:51 UTC (permalink / raw)
To: Cong Wang
Cc: cong.wang, john.fastabend, jakub, davem, edumazet, kuba, pabeni,
horms, andrii, eddyz87, mykolal, ast, daniel, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, shuah, mhal, sgarzare,
netdev, bpf, linux-kernel, linux-kselftest, mrpre
On Thu, Feb 27, 2025 at 11:52:04AM -0800, Cong Wang wrote:
> On Wed, Feb 26, 2025 at 09:22:41PM +0800, Jiayuan Chen wrote:
> > Current wrapper function create_pair() is used to create a pair of
> > connected links and returns two fds, but it does not support unix sockets.
> >
> > Here we introduce socketpair() into create_pair(), which supports creating
> > a pair of unix sockets, since the semantics of the two are the same.
>
> Since it is only for UDS and only has effectively 1 line of code, how
> about just calling socketpair(AF_UNIX) in your patch 3/3?
>
> Thanks!
Thanks, Cong, I'll expand selftest to cover TCP and UDP, looks like I
missed doing those.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH bpf-next v1 2/3] selftests/bpf: Add socketpair to create_pair to support unix socket
2025-02-27 22:21 ` John Fastabend
@ 2025-02-28 4:51 ` Jiayuan Chen
0 siblings, 0 replies; 14+ messages in thread
From: Jiayuan Chen @ 2025-02-28 4:51 UTC (permalink / raw)
To: John Fastabend
Cc: Cong Wang, cong.wang, jakub, davem, edumazet, kuba, pabeni, horms,
andrii, eddyz87, mykolal, ast, daniel, martin.lau, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, shuah, mhal, sgarzare,
netdev, bpf, linux-kernel, linux-kselftest, mrpre
On Thu, Feb 27, 2025 at 02:21:41PM -0800, John Fastabend wrote:
> On 2025-02-27 11:52:04, Cong Wang wrote:
> > On Wed, Feb 26, 2025 at 09:22:41PM +0800, Jiayuan Chen wrote:
> > > Current wrapper function create_pair() is used to create a pair of
> > > connected links and returns two fds, but it does not support unix sockets.
> > >
> > > Here we introduce socketpair() into create_pair(), which supports creating
> > > a pair of unix sockets, since the semantics of the two are the same.
> >
> > Since it is only for UDS and only has effectively 1 line of code, how
> > about just calling socketpair(AF_UNIX) in your patch 3/3?
>
> If we run that test with more than AF_UNIX it might be best as is. I
> think there might be some value testing that flow on TCP/UDP even if
> its not related to the bug.
>
> Thanks,
> John
Thanks, John! I'll make selftest cover more types of sockets.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-02-28 4:52 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-26 13:22 [PATCH bpf-next v1 0/3] bpf: Fix use-after-free of sockmap Jiayuan Chen
2025-02-26 13:22 ` [PATCH bpf-next v1 1/3] bpf, sockmap: avoid using sk_socket after free Jiayuan Chen
2025-02-27 19:45 ` Cong Wang
2025-02-27 20:57 ` John Fastabend
2025-02-27 23:04 ` Martin KaFai Lau
2025-02-28 4:49 ` Jiayuan Chen
2025-02-26 13:22 ` [PATCH bpf-next v1 2/3] selftests/bpf: Add socketpair to create_pair to support unix socket Jiayuan Chen
2025-02-27 19:52 ` Cong Wang
2025-02-27 22:21 ` John Fastabend
2025-02-28 4:51 ` Jiayuan Chen
2025-02-28 4:51 ` Jiayuan Chen
2025-02-26 13:22 ` [PATCH bpf-next v1 3/3] selftests/bpf: Add edge case tests for sockmap Jiayuan Chen
2025-02-27 19:53 ` Cong Wang
2025-02-27 22:18 ` John Fastabend
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).