* [PATCH net 1/4] sockmap, vsock: For connectible sockets allow only connected
2025-02-13 11:58 [PATCH net 0/4] sockmap, vsock: For connectible sockets allow only connected Michal Luczaj
@ 2025-02-13 11:58 ` Michal Luczaj
2025-02-14 13:11 ` Michal Luczaj
2025-02-13 11:58 ` [PATCH net 2/4] vsock/bpf: Warn on socket without transport Michal Luczaj
` (3 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Michal Luczaj @ 2025-02-13 11:58 UTC (permalink / raw)
To: John Fastabend, Jakub Sitnicki, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Stefano Garzarella, Michael S. Tsirkin,
Bobby Eshleman, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Mykola Lysenko, Shuah Khan
Cc: netdev, bpf, linux-kernel, linux-kselftest, Michal Luczaj
sockmap expects all vsocks to have a transport assigned, which is expressed
in vsock_proto::psock_update_sk_prot(). However, there is an edge case
where an unconnected (connectible) socket may lose its previously assigned
transport. This is handled with a NULL check in the vsock/BPF recv path.
Another design detail is that listening vsocks are not supposed to have any
transport assigned at all. Which implies they are not supported by the
sockmap. But this is complicated by the fact that a socket, before
switching to TCP_LISTEN, may have had some transport assigned during a
failed connect() attempt. Hence, we may end up with a listening vsock in a
sockmap, which blows up quickly:
KASAN: null-ptr-deref in range [0x0000000000000120-0x0000000000000127]
CPU: 7 UID: 0 PID: 56 Comm: kworker/7:0 Not tainted 6.14.0-rc1+
Workqueue: vsock-loopback vsock_loopback_work
RIP: 0010:vsock_read_skb+0x4b/0x90
Call Trace:
sk_psock_verdict_data_ready+0xa4/0x2e0
virtio_transport_recv_pkt+0x1ca8/0x2acc
vsock_loopback_work+0x27d/0x3f0
process_one_work+0x846/0x1420
worker_thread+0x5b3/0xf80
kthread+0x35a/0x700
ret_from_fork+0x2d/0x70
ret_from_fork_asm+0x1a/0x30
For connectible sockets, instead of relying solely on the state of
vsk->transport, tell sockmap to only allow those representing established
connections. This aligns with the behaviour for AF_INET and AF_UNIX.
Fixes: 634f1a7110b4 ("vsock: support sockmap")
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
net/core/sock_map.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index f1b9b3958792cd599efcb591742874e9b3f4a76b..2f1be9baad0578e2202b5cf79616b6e814c1ed54 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -541,6 +541,9 @@ static bool sock_map_sk_state_allowed(const struct sock *sk)
return (1 << sk->sk_state) & (TCPF_ESTABLISHED | TCPF_LISTEN);
if (sk_is_stream_unix(sk))
return (1 << sk->sk_state) & TCPF_ESTABLISHED;
+ if (sk_is_vsock(sk) &&
+ (sk->sk_type == SOCK_STREAM || sk->sk_type == SOCK_SEQPACKET))
+ return (1 << sk->sk_state) & TCPF_ESTABLISHED;
return true;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH net 1/4] sockmap, vsock: For connectible sockets allow only connected
2025-02-13 11:58 ` [PATCH net 1/4] " Michal Luczaj
@ 2025-02-14 13:11 ` Michal Luczaj
2025-02-18 8:52 ` Stefano Garzarella
0 siblings, 1 reply; 14+ messages in thread
From: Michal Luczaj @ 2025-02-14 13:11 UTC (permalink / raw)
To: John Fastabend, Jakub Sitnicki, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Stefano Garzarella, Michael S. Tsirkin,
Bobby Eshleman, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Mykola Lysenko, Shuah Khan
Cc: netdev, bpf, linux-kernel, linux-kselftest
> ...
> Another design detail is that listening vsocks are not supposed to have any
> transport assigned at all. Which implies they are not supported by the
> sockmap. But this is complicated by the fact that a socket, before
> switching to TCP_LISTEN, may have had some transport assigned during a
> failed connect() attempt. Hence, we may end up with a listening vsock in a
> sockmap, which blows up quickly:
>
> KASAN: null-ptr-deref in range [0x0000000000000120-0x0000000000000127]
> CPU: 7 UID: 0 PID: 56 Comm: kworker/7:0 Not tainted 6.14.0-rc1+
> Workqueue: vsock-loopback vsock_loopback_work
> RIP: 0010:vsock_read_skb+0x4b/0x90
> Call Trace:
> sk_psock_verdict_data_ready+0xa4/0x2e0
> virtio_transport_recv_pkt+0x1ca8/0x2acc
> vsock_loopback_work+0x27d/0x3f0
> process_one_work+0x846/0x1420
> worker_thread+0x5b3/0xf80
> kthread+0x35a/0x700
> ret_from_fork+0x2d/0x70
> ret_from_fork_asm+0x1a/0x30
Perhaps I should have expanded more on the null-ptr-deref itself.
The idea is: force a vsock into assigning a transport and add it to the
sockmap (with a verdict program), but keep it unconnected. Then, drop
the transport and set the vsock to TCP_LISTEN. The moment a new
connection is established:
virtio_transport_recv_pkt()
virtio_transport_recv_listen()
sk->sk_data_ready(sk) i.e. sk_psock_verdict_data_ready()
ops->read_skb() i.e. vsock_read_skb()
vsk->transport->read_skb() vsk->transport is NULL, boom
Here's a stand-alone repro:
/*
* # modprobe -a vsock_loopback vhost_vsock
* # gcc test.c && ./a.out
*/
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <sys/socket.h>
#include <sys/syscall.h>
#include <linux/bpf.h>
#include <linux/vm_sockets.h>
static void die(const char *msg)
{
perror(msg);
exit(-1);
}
static int sockmap_create(void)
{
union bpf_attr attr = {
.map_type = BPF_MAP_TYPE_SOCKMAP,
.key_size = sizeof(int),
.value_size = sizeof(int),
.max_entries = 1
};
int map;
map = syscall(SYS_bpf, BPF_MAP_CREATE, &attr, sizeof(attr));
if (map < 0)
die("map_create");
return map;
}
static void map_update_elem(int fd, int key, int value)
{
union bpf_attr attr = {
.map_fd = fd,
.key = (uint64_t)&key,
.value = (uint64_t)&value,
.flags = BPF_ANY
};
if (syscall(SYS_bpf, BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr)))
die("map_update_elem");
}
static int prog_load(void)
{
/* mov %r0, 1; exit */
struct bpf_insn insns[] = {
{ .code = BPF_ALU64 | BPF_MOV | BPF_K, .dst_reg = 0, .imm = 1 },
{ .code = BPF_JMP | BPF_EXIT }
};
union bpf_attr attr = {
.prog_type = BPF_PROG_TYPE_SK_SKB,
.insn_cnt = sizeof(insns)/sizeof(insns[0]),
.insns = (long)insns,
.license = (long)"",
};
int prog = syscall(SYS_bpf, BPF_PROG_LOAD, &attr, sizeof(attr));
if (prog < 0)
die("prog_load");
return prog;
}
static void link_create(int prog_fd, int target_fd)
{
union bpf_attr attr = {
.link_create = {
.prog_fd = prog_fd,
.target_fd = target_fd,
.attach_type = BPF_SK_SKB_VERDICT
}
};
if (syscall(SYS_bpf, BPF_LINK_CREATE, &attr, sizeof(attr)) < 0)
die("link_create");
}
int main(void)
{
struct sockaddr_vm addr = {
.svm_family = AF_VSOCK,
.svm_cid = VMADDR_CID_LOCAL,
.svm_port = VMADDR_PORT_ANY
};
socklen_t alen = sizeof(addr);
int s, map, prog, c;
s = socket(AF_VSOCK, SOCK_SEQPACKET, 0);
if (s < 0)
die("socket");
if (bind(s, (struct sockaddr *)&addr, alen))
die("bind");
if (!connect(s, (struct sockaddr *)&addr, alen) || errno != ECONNRESET)
die("connect #1");
map = sockmap_create();
prog = prog_load();
link_create(prog, map);
map_update_elem(map, 0, s);
addr.svm_cid = 0x42424242; /* non-existing */
if (!connect(s, (struct sockaddr *)&addr, alen) || errno != ESOCKTNOSUPPORT)
die("connect #2");
if (listen(s, 1))
die("listen");
if (getsockname(s, (struct sockaddr *)&addr, &alen))
die("getsockname");
c = socket(AF_VSOCK, SOCK_SEQPACKET, 0);
if (c < 0)
die("socket c");
if (connect(c, (struct sockaddr *)&addr, alen))
die("connect #3");
return 0;
}
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH net 1/4] sockmap, vsock: For connectible sockets allow only connected
2025-02-14 13:11 ` Michal Luczaj
@ 2025-02-18 8:52 ` Stefano Garzarella
0 siblings, 0 replies; 14+ messages in thread
From: Stefano Garzarella @ 2025-02-18 8:52 UTC (permalink / raw)
To: Michal Luczaj
Cc: John Fastabend, Jakub Sitnicki, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Michael S. Tsirkin, Bobby Eshleman,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
Shuah Khan, netdev, bpf, linux-kernel, linux-kselftest
On Fri, Feb 14, 2025 at 02:11:48PM +0100, Michal Luczaj wrote:
>> ...
>> Another design detail is that listening vsocks are not supposed to have any
>> transport assigned at all. Which implies they are not supported by the
>> sockmap. But this is complicated by the fact that a socket, before
>> switching to TCP_LISTEN, may have had some transport assigned during a
>> failed connect() attempt. Hence, we may end up with a listening vsock in a
>> sockmap, which blows up quickly:
>>
>> KASAN: null-ptr-deref in range [0x0000000000000120-0x0000000000000127]
>> CPU: 7 UID: 0 PID: 56 Comm: kworker/7:0 Not tainted 6.14.0-rc1+
>> Workqueue: vsock-loopback vsock_loopback_work
>> RIP: 0010:vsock_read_skb+0x4b/0x90
>> Call Trace:
>> sk_psock_verdict_data_ready+0xa4/0x2e0
>> virtio_transport_recv_pkt+0x1ca8/0x2acc
>> vsock_loopback_work+0x27d/0x3f0
>> process_one_work+0x846/0x1420
>> worker_thread+0x5b3/0xf80
>> kthread+0x35a/0x700
>> ret_from_fork+0x2d/0x70
>> ret_from_fork_asm+0x1a/0x30
>
>Perhaps I should have expanded more on the null-ptr-deref itself.
>
>The idea is: force a vsock into assigning a transport and add it to the
>sockmap (with a verdict program), but keep it unconnected. Then, drop
>the transport and set the vsock to TCP_LISTEN. The moment a new
>connection is established:
>
>virtio_transport_recv_pkt()
> virtio_transport_recv_listen()
> sk->sk_data_ready(sk) i.e. sk_psock_verdict_data_ready()
> ops->read_skb() i.e. vsock_read_skb()
> vsk->transport->read_skb() vsk->transport is NULL, boom
>
Yes I agree, it's a little clearer with this, but I think it was also
clear the concept before. So with or without:
Acked-by: Stefano Garzarella <sgarzare@redhat.com>
>Here's a stand-alone repro:
>
>/*
> * # modprobe -a vsock_loopback vhost_vsock
> * # gcc test.c && ./a.out
> */
>#include <stdio.h>
>#include <stdint.h>
>#include <stdlib.h>
>#include <unistd.h>
>#include <errno.h>
>#include <sys/socket.h>
>#include <sys/syscall.h>
>#include <linux/bpf.h>
>#include <linux/vm_sockets.h>
>
>static void die(const char *msg)
>{
> perror(msg);
> exit(-1);
>}
>
>static int sockmap_create(void)
>{
> union bpf_attr attr = {
> .map_type = BPF_MAP_TYPE_SOCKMAP,
> .key_size = sizeof(int),
> .value_size = sizeof(int),
> .max_entries = 1
> };
> int map;
>
> map = syscall(SYS_bpf, BPF_MAP_CREATE, &attr, sizeof(attr));
> if (map < 0)
> die("map_create");
>
> return map;
>}
>
>static void map_update_elem(int fd, int key, int value)
>{
> union bpf_attr attr = {
> .map_fd = fd,
> .key = (uint64_t)&key,
> .value = (uint64_t)&value,
> .flags = BPF_ANY
> };
>
> if (syscall(SYS_bpf, BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr)))
> die("map_update_elem");
>}
>
>static int prog_load(void)
>{
> /* mov %r0, 1; exit */
> struct bpf_insn insns[] = {
> { .code = BPF_ALU64 | BPF_MOV | BPF_K, .dst_reg = 0, .imm = 1 },
> { .code = BPF_JMP | BPF_EXIT }
> };
> union bpf_attr attr = {
> .prog_type = BPF_PROG_TYPE_SK_SKB,
> .insn_cnt = sizeof(insns)/sizeof(insns[0]),
> .insns = (long)insns,
> .license = (long)"",
> };
>
> int prog = syscall(SYS_bpf, BPF_PROG_LOAD, &attr, sizeof(attr));
> if (prog < 0)
> die("prog_load");
>
> return prog;
>}
>
>static void link_create(int prog_fd, int target_fd)
>{
> union bpf_attr attr = {
> .link_create = {
> .prog_fd = prog_fd,
> .target_fd = target_fd,
> .attach_type = BPF_SK_SKB_VERDICT
> }
> };
>
> if (syscall(SYS_bpf, BPF_LINK_CREATE, &attr, sizeof(attr)) < 0)
> die("link_create");
>}
>
>int main(void)
>{
> struct sockaddr_vm addr = {
> .svm_family = AF_VSOCK,
> .svm_cid = VMADDR_CID_LOCAL,
> .svm_port = VMADDR_PORT_ANY
> };
> socklen_t alen = sizeof(addr);
> int s, map, prog, c;
>
> s = socket(AF_VSOCK, SOCK_SEQPACKET, 0);
> if (s < 0)
> die("socket");
>
> if (bind(s, (struct sockaddr *)&addr, alen))
> die("bind");
>
> if (!connect(s, (struct sockaddr *)&addr, alen) || errno != ECONNRESET)
> die("connect #1");
>
> map = sockmap_create();
> prog = prog_load();
> link_create(prog, map);
> map_update_elem(map, 0, s);
>
> addr.svm_cid = 0x42424242; /* non-existing */
> if (!connect(s, (struct sockaddr *)&addr, alen) || errno != ESOCKTNOSUPPORT)
> die("connect #2");
>
> if (listen(s, 1))
> die("listen");
>
> if (getsockname(s, (struct sockaddr *)&addr, &alen))
> die("getsockname");
>
> c = socket(AF_VSOCK, SOCK_SEQPACKET, 0);
> if (c < 0)
> die("socket c");
>
> if (connect(c, (struct sockaddr *)&addr, alen))
> die("connect #3");
>
> return 0;
>}
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net 2/4] vsock/bpf: Warn on socket without transport
2025-02-13 11:58 [PATCH net 0/4] sockmap, vsock: For connectible sockets allow only connected Michal Luczaj
2025-02-13 11:58 ` [PATCH net 1/4] " Michal Luczaj
@ 2025-02-13 11:58 ` Michal Luczaj
2025-02-17 10:59 ` Stefano Garzarella
2025-02-13 11:58 ` [PATCH net 3/4] selftest/bpf: Adapt vsock_delete_on_close to sockmap rejecting unconnected Michal Luczaj
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Michal Luczaj @ 2025-02-13 11:58 UTC (permalink / raw)
To: John Fastabend, Jakub Sitnicki, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Stefano Garzarella, Michael S. Tsirkin,
Bobby Eshleman, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Mykola Lysenko, Shuah Khan
Cc: netdev, bpf, linux-kernel, linux-kselftest, Michal Luczaj
In the spirit of commit 91751e248256 ("vsock: prevent null-ptr-deref in
vsock_*[has_data|has_space]"), armorize the "impossible" cases with a
warning.
Fixes: 634f1a7110b4 ("vsock: support sockmap")
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
net/vmw_vsock/af_vsock.c | 3 +++
net/vmw_vsock/vsock_bpf.c | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 53a081d49d28ac1c04e7f8057c8a55e7b73cc131..7e3db87ae4333cf63327ec105ca99253569bb9fe 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1189,6 +1189,9 @@ static int vsock_read_skb(struct sock *sk, skb_read_actor_t read_actor)
{
struct vsock_sock *vsk = vsock_sk(sk);
+ if (WARN_ON_ONCE(!vsk->transport))
+ return -ENODEV;
+
return vsk->transport->read_skb(vsk, read_actor);
}
diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
index f201d9eca1df2f8143638cf7a4d08671e8368c11..07b96d56f3a577af71021b1b8132743554996c4f 100644
--- a/net/vmw_vsock/vsock_bpf.c
+++ b/net/vmw_vsock/vsock_bpf.c
@@ -87,7 +87,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
lock_sock(sk);
vsk = vsock_sk(sk);
- if (!vsk->transport) {
+ if (WARN_ON_ONCE(!vsk->transport)) {
copied = -ENODEV;
goto out;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH net 2/4] vsock/bpf: Warn on socket without transport
2025-02-13 11:58 ` [PATCH net 2/4] vsock/bpf: Warn on socket without transport Michal Luczaj
@ 2025-02-17 10:59 ` Stefano Garzarella
2025-02-17 19:45 ` Michal Luczaj
0 siblings, 1 reply; 14+ messages in thread
From: Stefano Garzarella @ 2025-02-17 10:59 UTC (permalink / raw)
To: Michal Luczaj
Cc: John Fastabend, Jakub Sitnicki, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Michael S. Tsirkin, Bobby Eshleman,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
Shuah Khan, netdev, bpf, linux-kernel, linux-kselftest
On Thu, Feb 13, 2025 at 12:58:50PM +0100, Michal Luczaj wrote:
>In the spirit of commit 91751e248256 ("vsock: prevent null-ptr-deref in
>vsock_*[has_data|has_space]"), armorize the "impossible" cases with a
>warning.
>
>Fixes: 634f1a7110b4 ("vsock: support sockmap")
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> net/vmw_vsock/af_vsock.c | 3 +++
> net/vmw_vsock/vsock_bpf.c | 2 +-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 53a081d49d28ac1c04e7f8057c8a55e7b73cc131..7e3db87ae4333cf63327ec105ca99253569bb9fe 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1189,6 +1189,9 @@ static int vsock_read_skb(struct sock *sk, skb_read_actor_t read_actor)
> {
> struct vsock_sock *vsk = vsock_sk(sk);
>
>+ if (WARN_ON_ONCE(!vsk->transport))
>+ return -ENODEV;
>+
> return vsk->transport->read_skb(vsk, read_actor);
> }
>
>diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
>index f201d9eca1df2f8143638cf7a4d08671e8368c11..07b96d56f3a577af71021b1b8132743554996c4f 100644
>--- a/net/vmw_vsock/vsock_bpf.c
>+++ b/net/vmw_vsock/vsock_bpf.c
>@@ -87,7 +87,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
> lock_sock(sk);
> vsk = vsock_sk(sk);
>
>- if (!vsk->transport) {
>+ if (WARN_ON_ONCE(!vsk->transport)) {
> copied = -ENODEV;
> goto out;
> }
I'm not a sockmap expert, so I don't understand why here print an
error.
Since there was already a check, I expected it to be a case that can
happen, but instead calling `rcvmsg()` on a socket not yet connected is
impossible?
Thanks,
Stefano
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH net 2/4] vsock/bpf: Warn on socket without transport
2025-02-17 10:59 ` Stefano Garzarella
@ 2025-02-17 19:45 ` Michal Luczaj
2025-02-18 8:49 ` Stefano Garzarella
0 siblings, 1 reply; 14+ messages in thread
From: Michal Luczaj @ 2025-02-17 19:45 UTC (permalink / raw)
To: Stefano Garzarella
Cc: John Fastabend, Jakub Sitnicki, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Michael S. Tsirkin, Bobby Eshleman,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
Shuah Khan, netdev, bpf, linux-kernel, linux-kselftest
On 2/17/25 11:59, Stefano Garzarella wrote:
> On Thu, Feb 13, 2025 at 12:58:50PM +0100, Michal Luczaj wrote:
>> In the spirit of commit 91751e248256 ("vsock: prevent null-ptr-deref in
>> vsock_*[has_data|has_space]"), armorize the "impossible" cases with a
>> warning.
>>
>> Fixes: 634f1a7110b4 ("vsock: support sockmap")
>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>> ---
>> net/vmw_vsock/af_vsock.c | 3 +++
>> net/vmw_vsock/vsock_bpf.c | 2 +-
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index 53a081d49d28ac1c04e7f8057c8a55e7b73cc131..7e3db87ae4333cf63327ec105ca99253569bb9fe 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -1189,6 +1189,9 @@ static int vsock_read_skb(struct sock *sk, skb_read_actor_t read_actor)
>> {
>> struct vsock_sock *vsk = vsock_sk(sk);
>>
>> + if (WARN_ON_ONCE(!vsk->transport))
>> + return -ENODEV;
>> +
>> return vsk->transport->read_skb(vsk, read_actor);
>> }
>>
>> diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
>> index f201d9eca1df2f8143638cf7a4d08671e8368c11..07b96d56f3a577af71021b1b8132743554996c4f 100644
>> --- a/net/vmw_vsock/vsock_bpf.c
>> +++ b/net/vmw_vsock/vsock_bpf.c
>> @@ -87,7 +87,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
>> lock_sock(sk);
>> vsk = vsock_sk(sk);
>>
>> - if (!vsk->transport) {
>> + if (WARN_ON_ONCE(!vsk->transport)) {
>> copied = -ENODEV;
>> goto out;
>> }
>
> I'm not a sockmap expert, so I don't understand why here print an
> error.
>
> Since there was already a check, I expected it to be a case that can
> happen, but instead calling `rcvmsg()` on a socket not yet connected is
> impossible?
That's right, calling vsock_bpf_recvmsg() on a not-yet-connected
connectible socket is impossible since PATCH 1/4 of this series.
That is because to reach vsock_bpf_recvmsg(), you must have sock's proto
replaced in vsock_bpf_update_proto(). For that you need to run
sock_map_init_proto(), which you can't because the patched
sock_map_sk_state_allowed() will stop you.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH net 2/4] vsock/bpf: Warn on socket without transport
2025-02-17 19:45 ` Michal Luczaj
@ 2025-02-18 8:49 ` Stefano Garzarella
0 siblings, 0 replies; 14+ messages in thread
From: Stefano Garzarella @ 2025-02-18 8:49 UTC (permalink / raw)
To: Michal Luczaj
Cc: John Fastabend, Jakub Sitnicki, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Michael S. Tsirkin, Bobby Eshleman,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
Shuah Khan, netdev, bpf, linux-kernel, linux-kselftest
On Mon, Feb 17, 2025 at 08:45:06PM +0100, Michal Luczaj wrote:
>On 2/17/25 11:59, Stefano Garzarella wrote:
>> On Thu, Feb 13, 2025 at 12:58:50PM +0100, Michal Luczaj wrote:
>>> In the spirit of commit 91751e248256 ("vsock: prevent null-ptr-deref in
>>> vsock_*[has_data|has_space]"), armorize the "impossible" cases with a
>>> warning.
>>>
>>> Fixes: 634f1a7110b4 ("vsock: support sockmap")
>>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>>> ---
>>> net/vmw_vsock/af_vsock.c | 3 +++
>>> net/vmw_vsock/vsock_bpf.c | 2 +-
>>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>>> index 53a081d49d28ac1c04e7f8057c8a55e7b73cc131..7e3db87ae4333cf63327ec105ca99253569bb9fe 100644
>>> --- a/net/vmw_vsock/af_vsock.c
>>> +++ b/net/vmw_vsock/af_vsock.c
>>> @@ -1189,6 +1189,9 @@ static int vsock_read_skb(struct sock *sk, skb_read_actor_t read_actor)
>>> {
>>> struct vsock_sock *vsk = vsock_sk(sk);
>>>
>>> + if (WARN_ON_ONCE(!vsk->transport))
>>> + return -ENODEV;
>>> +
>>> return vsk->transport->read_skb(vsk, read_actor);
>>> }
>>>
>>> diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
>>> index f201d9eca1df2f8143638cf7a4d08671e8368c11..07b96d56f3a577af71021b1b8132743554996c4f 100644
>>> --- a/net/vmw_vsock/vsock_bpf.c
>>> +++ b/net/vmw_vsock/vsock_bpf.c
>>> @@ -87,7 +87,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
>>> lock_sock(sk);
>>> vsk = vsock_sk(sk);
>>>
>>> - if (!vsk->transport) {
>>> + if (WARN_ON_ONCE(!vsk->transport)) {
>>> copied = -ENODEV;
>>> goto out;
>>> }
>>
>> I'm not a sockmap expert, so I don't understand why here print an
>> error.
>>
>> Since there was already a check, I expected it to be a case that can
>> happen, but instead calling `rcvmsg()` on a socket not yet connected is
>> impossible?
>
>That's right, calling vsock_bpf_recvmsg() on a not-yet-connected
>connectible socket is impossible since PATCH 1/4 of this series.
Okay, I get it now.
If you need to send a v2, can you write it in the commit description
that the behavior changed in the previous patch and so in
vsock_bpf_recvmsg() now we no longer expect to be called with a null
transport.
In any case, LGTM:
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
>That is because to reach vsock_bpf_recvmsg(), you must have sock's proto
>replaced in vsock_bpf_update_proto(). For that you need to run
>sock_map_init_proto(), which you can't because the patched
>sock_map_sk_state_allowed() will stop you.
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net 3/4] selftest/bpf: Adapt vsock_delete_on_close to sockmap rejecting unconnected
2025-02-13 11:58 [PATCH net 0/4] sockmap, vsock: For connectible sockets allow only connected Michal Luczaj
2025-02-13 11:58 ` [PATCH net 1/4] " Michal Luczaj
2025-02-13 11:58 ` [PATCH net 2/4] vsock/bpf: Warn on socket without transport Michal Luczaj
@ 2025-02-13 11:58 ` Michal Luczaj
2025-02-18 8:53 ` Stefano Garzarella
2025-02-13 11:58 ` [PATCH net 4/4] selftest/bpf: Add vsock test for " Michal Luczaj
2025-02-18 11:10 ` [PATCH net 0/4] sockmap, vsock: For connectible sockets allow only connected patchwork-bot+netdevbpf
4 siblings, 1 reply; 14+ messages in thread
From: Michal Luczaj @ 2025-02-13 11:58 UTC (permalink / raw)
To: John Fastabend, Jakub Sitnicki, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Stefano Garzarella, Michael S. Tsirkin,
Bobby Eshleman, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Mykola Lysenko, Shuah Khan
Cc: netdev, bpf, linux-kernel, linux-kselftest, Michal Luczaj
Commit 515745445e92 ("selftest/bpf: Add test for vsock removal from sockmap
on close()") added test that checked if proto::close() callback was invoked
on AF_VSOCK socket release. I.e. it verified that a close()d vsock does
indeed get removed from the sockmap.
It was done simply by creating a socket pair and attempting to replace a
close()d one with its peer. Since, due to a recent change, sockmap does not
allow updating index with a non-established connectible vsock, redo it with
a freshly established one.
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
.../selftests/bpf/prog_tests/sockmap_basic.c | 40 ++++++++++++----------
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 884ad87783d59ef3d1ca84c3a542f3f8670cd463..21793d8c79e12b6e607f59ecebb26448c310044b 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -111,31 +111,35 @@ static void test_sockmap_create_update_free(enum bpf_map_type map_type)
static void test_sockmap_vsock_delete_on_close(void)
{
- int err, c, p, map;
- const int zero = 0;
-
- err = create_pair(AF_VSOCK, SOCK_STREAM, &c, &p);
- if (!ASSERT_OK(err, "create_pair(AF_VSOCK)"))
- return;
+ int map, c, p, err, zero = 0;
map = bpf_map_create(BPF_MAP_TYPE_SOCKMAP, NULL, sizeof(int),
sizeof(int), 1, NULL);
- if (!ASSERT_GE(map, 0, "bpf_map_create")) {
- close(c);
- goto out;
- }
+ if (!ASSERT_OK_FD(map, "bpf_map_create"))
+ return;
- err = bpf_map_update_elem(map, &zero, &c, BPF_NOEXIST);
- close(c);
- if (!ASSERT_OK(err, "bpf_map_update"))
- goto out;
+ err = create_pair(AF_VSOCK, SOCK_STREAM, &c, &p);
+ if (!ASSERT_OK(err, "create_pair"))
+ goto close_map;
- err = bpf_map_update_elem(map, &zero, &p, BPF_NOEXIST);
+ if (xbpf_map_update_elem(map, &zero, &c, BPF_NOEXIST))
+ goto close_socks;
+
+ xclose(c);
+ xclose(p);
+
+ err = create_pair(AF_VSOCK, SOCK_STREAM, &c, &p);
+ if (!ASSERT_OK(err, "create_pair"))
+ goto close_map;
+
+ err = bpf_map_update_elem(map, &zero, &c, BPF_NOEXIST);
ASSERT_OK(err, "after close(), bpf_map_update");
-out:
- close(p);
- close(map);
+close_socks:
+ xclose(c);
+ xclose(p);
+close_map:
+ xclose(map);
}
static void test_skmsg_helpers(enum bpf_map_type map_type)
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH net 3/4] selftest/bpf: Adapt vsock_delete_on_close to sockmap rejecting unconnected
2025-02-13 11:58 ` [PATCH net 3/4] selftest/bpf: Adapt vsock_delete_on_close to sockmap rejecting unconnected Michal Luczaj
@ 2025-02-18 8:53 ` Stefano Garzarella
0 siblings, 0 replies; 14+ messages in thread
From: Stefano Garzarella @ 2025-02-18 8:53 UTC (permalink / raw)
To: Michal Luczaj
Cc: John Fastabend, Jakub Sitnicki, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Michael S. Tsirkin, Bobby Eshleman,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
Shuah Khan, netdev, bpf, linux-kernel, linux-kselftest
On Thu, Feb 13, 2025 at 12:58:51PM +0100, Michal Luczaj wrote:
>Commit 515745445e92 ("selftest/bpf: Add test for vsock removal from sockmap
>on close()") added test that checked if proto::close() callback was invoked
>on AF_VSOCK socket release. I.e. it verified that a close()d vsock does
>indeed get removed from the sockmap.
>
>It was done simply by creating a socket pair and attempting to replace a
>close()d one with its peer. Since, due to a recent change, sockmap does not
>allow updating index with a non-established connectible vsock, redo it with
>a freshly established one.
>
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> .../selftests/bpf/prog_tests/sockmap_basic.c | 40 ++++++++++++----------
> 1 file changed, 22 insertions(+), 18 deletions(-)
Acked-by: Stefano Garzarella <sgarzare@redhat.com>
>
>diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
>index 884ad87783d59ef3d1ca84c3a542f3f8670cd463..21793d8c79e12b6e607f59ecebb26448c310044b 100644
>--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
>+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
>@@ -111,31 +111,35 @@ static void test_sockmap_create_update_free(enum bpf_map_type map_type)
>
> static void test_sockmap_vsock_delete_on_close(void)
> {
>- int err, c, p, map;
>- const int zero = 0;
>-
>- err = create_pair(AF_VSOCK, SOCK_STREAM, &c, &p);
>- if (!ASSERT_OK(err, "create_pair(AF_VSOCK)"))
>- return;
>+ int map, c, p, err, zero = 0;
>
> map = bpf_map_create(BPF_MAP_TYPE_SOCKMAP, NULL, sizeof(int),
> sizeof(int), 1, NULL);
>- if (!ASSERT_GE(map, 0, "bpf_map_create")) {
>- close(c);
>- goto out;
>- }
>+ if (!ASSERT_OK_FD(map, "bpf_map_create"))
>+ return;
>
>- err = bpf_map_update_elem(map, &zero, &c, BPF_NOEXIST);
>- close(c);
>- if (!ASSERT_OK(err, "bpf_map_update"))
>- goto out;
>+ err = create_pair(AF_VSOCK, SOCK_STREAM, &c, &p);
>+ if (!ASSERT_OK(err, "create_pair"))
>+ goto close_map;
>
>- err = bpf_map_update_elem(map, &zero, &p, BPF_NOEXIST);
>+ if (xbpf_map_update_elem(map, &zero, &c, BPF_NOEXIST))
>+ goto close_socks;
>+
>+ xclose(c);
>+ xclose(p);
>+
>+ err = create_pair(AF_VSOCK, SOCK_STREAM, &c, &p);
>+ if (!ASSERT_OK(err, "create_pair"))
>+ goto close_map;
>+
>+ err = bpf_map_update_elem(map, &zero, &c, BPF_NOEXIST);
> ASSERT_OK(err, "after close(), bpf_map_update");
>
>-out:
>- close(p);
>- close(map);
>+close_socks:
>+ xclose(c);
>+ xclose(p);
>+close_map:
>+ xclose(map);
> }
>
> static void test_skmsg_helpers(enum bpf_map_type map_type)
>
>--
>2.48.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net 4/4] selftest/bpf: Add vsock test for sockmap rejecting unconnected
2025-02-13 11:58 [PATCH net 0/4] sockmap, vsock: For connectible sockets allow only connected Michal Luczaj
` (2 preceding siblings ...)
2025-02-13 11:58 ` [PATCH net 3/4] selftest/bpf: Adapt vsock_delete_on_close to sockmap rejecting unconnected Michal Luczaj
@ 2025-02-13 11:58 ` Michal Luczaj
2025-02-14 13:12 ` Michal Luczaj
2025-02-18 8:54 ` Stefano Garzarella
2025-02-18 11:10 ` [PATCH net 0/4] sockmap, vsock: For connectible sockets allow only connected patchwork-bot+netdevbpf
4 siblings, 2 replies; 14+ messages in thread
From: Michal Luczaj @ 2025-02-13 11:58 UTC (permalink / raw)
To: John Fastabend, Jakub Sitnicki, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Stefano Garzarella, Michael S. Tsirkin,
Bobby Eshleman, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Mykola Lysenko, Shuah Khan
Cc: netdev, bpf, linux-kernel, linux-kselftest, Michal Luczaj
Verify that for a connectible AF_VSOCK socket, merely having a transport
assigned is insufficient; socket must be connected for the sockmap to
accept.
This does not test datagram vsocks. Even though it hardly matters. VMCI is
the only transport that features VSOCK_TRANSPORT_F_DGRAM, but it has an
unimplemented vsock_transport::readskb() callback, making it unsupported by
BPF/sockmap.
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
.../selftests/bpf/prog_tests/sockmap_basic.c | 30 ++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index 21793d8c79e12b6e607f59ecebb26448c310044b..05eb37935c3e290ee52b8d8c7c3e3a8db026cba2 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -1065,6 +1065,34 @@ static void test_sockmap_skb_verdict_vsock_poll(void)
test_sockmap_pass_prog__destroy(skel);
}
+static void test_sockmap_vsock_unconnected(void)
+{
+ struct sockaddr_storage addr;
+ int map, s, zero = 0;
+ socklen_t alen;
+
+ map = bpf_map_create(BPF_MAP_TYPE_SOCKMAP, NULL, sizeof(int),
+ sizeof(int), 1, NULL);
+ if (!ASSERT_OK_FD(map, "bpf_map_create"))
+ return;
+
+ s = xsocket(AF_VSOCK, SOCK_STREAM, 0);
+ if (s < 0)
+ goto close_map;
+
+ /* Fail connect(), but trigger transport assignment. */
+ init_addr_loopback(AF_VSOCK, &addr, &alen);
+ if (!ASSERT_ERR(connect(s, sockaddr(&addr), alen), "connect"))
+ goto close_sock;
+
+ ASSERT_ERR(bpf_map_update_elem(map, &zero, &s, BPF_ANY), "map_update");
+
+close_sock:
+ xclose(s);
+close_map:
+ xclose(map);
+}
+
void test_sockmap_basic(void)
{
if (test__start_subtest("sockmap create_update_free"))
@@ -1131,4 +1159,6 @@ void test_sockmap_basic(void)
test_skmsg_helpers_with_link(BPF_MAP_TYPE_SOCKHASH);
if (test__start_subtest("sockmap skb_verdict vsock poll"))
test_sockmap_skb_verdict_vsock_poll();
+ if (test__start_subtest("sockmap vsock unconnected"))
+ test_sockmap_vsock_unconnected();
}
--
2.48.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH net 4/4] selftest/bpf: Add vsock test for sockmap rejecting unconnected
2025-02-13 11:58 ` [PATCH net 4/4] selftest/bpf: Add vsock test for " Michal Luczaj
@ 2025-02-14 13:12 ` Michal Luczaj
2025-02-18 8:54 ` Stefano Garzarella
1 sibling, 0 replies; 14+ messages in thread
From: Michal Luczaj @ 2025-02-14 13:12 UTC (permalink / raw)
To: John Fastabend, Jakub Sitnicki, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Stefano Garzarella, Michael S. Tsirkin,
Bobby Eshleman, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Mykola Lysenko, Shuah Khan
Cc: netdev, bpf, linux-kernel, linux-kselftest
On 2/13/25 12:58, Michal Luczaj wrote:
> ...
> This does not test datagram vsocks. Even though it hardly matters. VMCI is
> the only transport that features VSOCK_TRANSPORT_F_DGRAM, but it has an
> unimplemented vsock_transport::readskb() callback, making it unsupported by
^^^^^^^
Ugh, read_skb().
Sorry,
Michal
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH net 4/4] selftest/bpf: Add vsock test for sockmap rejecting unconnected
2025-02-13 11:58 ` [PATCH net 4/4] selftest/bpf: Add vsock test for " Michal Luczaj
2025-02-14 13:12 ` Michal Luczaj
@ 2025-02-18 8:54 ` Stefano Garzarella
1 sibling, 0 replies; 14+ messages in thread
From: Stefano Garzarella @ 2025-02-18 8:54 UTC (permalink / raw)
To: Michal Luczaj
Cc: John Fastabend, Jakub Sitnicki, Eric Dumazet, Kuniyuki Iwashima,
Paolo Abeni, Willem de Bruijn, David S. Miller, Jakub Kicinski,
Simon Horman, Michael S. Tsirkin, Bobby Eshleman,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Mykola Lysenko,
Shuah Khan, netdev, bpf, linux-kernel, linux-kselftest
On Thu, Feb 13, 2025 at 12:58:52PM +0100, Michal Luczaj wrote:
>Verify that for a connectible AF_VSOCK socket, merely having a transport
>assigned is insufficient; socket must be connected for the sockmap to
>accept.
>
>This does not test datagram vsocks. Even though it hardly matters. VMCI is
>the only transport that features VSOCK_TRANSPORT_F_DGRAM, but it has an
>unimplemented vsock_transport::readskb() callback, making it unsupported by
>BPF/sockmap.
>
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> .../selftests/bpf/prog_tests/sockmap_basic.c | 30 ++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
Acked-by: Stefano Garzarella <sgarzare@redhat.com>
>
>diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
>index 21793d8c79e12b6e607f59ecebb26448c310044b..05eb37935c3e290ee52b8d8c7c3e3a8db026cba2 100644
>--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
>+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
>@@ -1065,6 +1065,34 @@ static void test_sockmap_skb_verdict_vsock_poll(void)
> test_sockmap_pass_prog__destroy(skel);
> }
>
>+static void test_sockmap_vsock_unconnected(void)
>+{
>+ struct sockaddr_storage addr;
>+ int map, s, zero = 0;
>+ socklen_t alen;
>+
>+ map = bpf_map_create(BPF_MAP_TYPE_SOCKMAP, NULL, sizeof(int),
>+ sizeof(int), 1, NULL);
>+ if (!ASSERT_OK_FD(map, "bpf_map_create"))
>+ return;
>+
>+ s = xsocket(AF_VSOCK, SOCK_STREAM, 0);
>+ if (s < 0)
>+ goto close_map;
>+
>+ /* Fail connect(), but trigger transport assignment. */
>+ init_addr_loopback(AF_VSOCK, &addr, &alen);
>+ if (!ASSERT_ERR(connect(s, sockaddr(&addr), alen), "connect"))
>+ goto close_sock;
>+
>+ ASSERT_ERR(bpf_map_update_elem(map, &zero, &s, BPF_ANY), "map_update");
>+
>+close_sock:
>+ xclose(s);
>+close_map:
>+ xclose(map);
>+}
>+
> void test_sockmap_basic(void)
> {
> if (test__start_subtest("sockmap create_update_free"))
>@@ -1131,4 +1159,6 @@ void test_sockmap_basic(void)
> test_skmsg_helpers_with_link(BPF_MAP_TYPE_SOCKHASH);
> if (test__start_subtest("sockmap skb_verdict vsock poll"))
> test_sockmap_skb_verdict_vsock_poll();
>+ if (test__start_subtest("sockmap vsock unconnected"))
>+ test_sockmap_vsock_unconnected();
> }
>
>--
>2.48.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net 0/4] sockmap, vsock: For connectible sockets allow only connected
2025-02-13 11:58 [PATCH net 0/4] sockmap, vsock: For connectible sockets allow only connected Michal Luczaj
` (3 preceding siblings ...)
2025-02-13 11:58 ` [PATCH net 4/4] selftest/bpf: Add vsock test for " Michal Luczaj
@ 2025-02-18 11:10 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 14+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-02-18 11:10 UTC (permalink / raw)
To: Michal Luczaj
Cc: john.fastabend, jakub, edumazet, kuniyu, pabeni, willemb, davem,
kuba, horms, sgarzare, mst, bobby.eshleman, ast, daniel, andrii,
martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf, haoluo,
jolsa, mykolal, shuah, netdev, bpf, linux-kernel, linux-kselftest
Hello:
This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Thu, 13 Feb 2025 12:58:48 +0100 you wrote:
> Series deals with one more case of vsock surprising BPF/sockmap by being
> inconsistency about (having an) assigned transport.
>
> KASAN: null-ptr-deref in range [0x0000000000000120-0x0000000000000127]
> CPU: 7 UID: 0 PID: 56 Comm: kworker/7:0 Not tainted 6.14.0-rc1+
> Workqueue: vsock-loopback vsock_loopback_work
> RIP: 0010:vsock_read_skb+0x4b/0x90
> Call Trace:
> sk_psock_verdict_data_ready+0xa4/0x2e0
> virtio_transport_recv_pkt+0x1ca8/0x2acc
> vsock_loopback_work+0x27d/0x3f0
> process_one_work+0x846/0x1420
> worker_thread+0x5b3/0xf80
> kthread+0x35a/0x700
> ret_from_fork+0x2d/0x70
> ret_from_fork_asm+0x1a/0x30
>
> [...]
Here is the summary with links:
- [net,1/4] sockmap, vsock: For connectible sockets allow only connected
https://git.kernel.org/netdev/net/c/8fb5bb169d17
- [net,2/4] vsock/bpf: Warn on socket without transport
https://git.kernel.org/netdev/net/c/857ae05549ee
- [net,3/4] selftest/bpf: Adapt vsock_delete_on_close to sockmap rejecting unconnected
https://git.kernel.org/netdev/net/c/8350695bfb16
- [net,4/4] selftest/bpf: Add vsock test for sockmap rejecting unconnected
https://git.kernel.org/netdev/net/c/85928e9c4363
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 14+ messages in thread