Re: [PATCH net v3 2/2] vsock/bpf: fix zero-length, MSG_OOB and MSG_ERRQUEUE reads in recvmsg

[Stefano Garzarella]

On Wed, Oct 28, 2026 at 06:55:41PM +0100, Jerome Mohm via B4 Relay wrote:
> From: Jerome Mohm <jrmmhm.kernel@xxxxxxxxx>
>
> vsock_bpf_recvmsg() sends every read that cannot be served from the
> transport's own queue through sk_msg_recvmsg() and its wait loop. Three
> kinds of read do not fit that path:
>
> - len == 0: with data queued in the psock ingress queue (for example
>   after an SK_PASS verdict), sk_msg_recvmsg() copies nothing and
>   returns 0, and the wait reports data ready at once, so the loop spins
>   with lock_sock() held. The vsock delivery worker then blocks on the
>   same lock and all rx on the transport stops (hung-task).
>   tcp_bpf, udp_bpf and unix_bpf return 0 early, and
>   __vsock_connectible_recvmsg() handles len == 0 too.
>
> - MSG_ERRQUEUE: the read only reaches sock_recv_errqueue() when the
>   native fallback happens to be taken. With empty queues a blocking
>   recvmsg(MSG_ERRQUEUE) waits for stream data instead of returning
>   -EAGAIN for an empty error queue. Route it to the native path first,
>   as tcp_bpf does, so it is also not caught by the len == 0 check.
>
> - MSG_OOB: vsock has no out-of-band data and the native path returns
>   -EOPNOTSUPP, but sk_msg_recvmsg() ignores the flag and consumes
>   ordinary stream bytes; with empty queues the read waits for data.
>   Reject it right after the MSG_ERRQUEUE route and, as
>   unix_bpf_recvmsg() does, before the len == 0 check.
>
> The len == 0 and MSG_OOB cases were reported by the netdev Sashiko AI
> review of earlier, single-patch versions of this fix; the MSG_ERRQUEUE
> case was found with the help of an LLM coding assistant (Anthropic's
> Claude). Each reproducer is a C program run as root in the guest: it
> adds one end of a vsock-loopback stream connection to a BPF sockmap
> (CAP_BPF and CAP_NET_ADMIN), for the queued-byte cases with an SK_PASS
> stream-verdict program, and issues the reads below. No kernel change or
> error injection is needed. The reproducers are available on request.
>
> Testing: on a private VM with a KASAN + lockdep kernel on net, with
> reproducers over vsock-loopback. Before this patch: recv(fd, buf, 0, 0)
> with a byte in the psock ingress queue spins and the delivery worker
> hangs (hung-task); a zero-length recvmsg(MSG_ERRQUEUE) blocks; and
> recv() with MSG_OOB waits for data or returns a queued ordinary byte,
> which a following plain recv() then misses. After: a zero-length recv()
> returns 0 and the next recv() returns the byte; MSG_ERRQUEUE returns
> -EAGAIN from the empty error queue; MSG_OOB returns -EOPNOTSUPP at once
> and consumes nothing. With both patches of this series applied, the
> three single-VM tests of tools/testing/selftests/vsock/vmtest.sh
> (vm_server_host_client, vm_client_host_server, vm_loopback; 44
> vsock_test cases each) pass, as on net; on both kernels the host-side
> runs skip the SO_INQ cases (SO_INQ not supported by the host kernel).
> The ns_* tests of vmtest.sh were not run (they need root on the host).
> The BPF test_progs sockmap_basic, sockmap_listen and sockmap_redir tests
> pass (767 of 767 subtests, among them the three vsock subtests of
> sockmap_basic and the 240 AF_VSOCK cases of sockmap_redir).

Ditto.

> Fixes: 634f1a7110b4 ("vsock: support sockmap")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jerome Mohm <jrmmhm.kernel@xxxxxxxxx>
> ---
> net/vmw_vsock/vsock_bpf.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
> index 5120911b8143..3edc01268c42 100644
> --- a/net/vmw_vsock/vsock_bpf.c
> +++ b/net/vmw_vsock/vsock_bpf.c
> @@ -111,6 +111,15 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
> 	struct vsock_sock *vsk;
> 	int copied;
>
> +	if (unlikely(flags & MSG_ERRQUEUE))
> +		return __vsock_recvmsg(sk, msg, len, flags);

If the psock ingress queue is not empty, wouldn't recvmsg(MSG_ERRQUEUE) 
(e.g. a MSG_ZEROCOPY user reading completions) also consume stream bytes 
through sk_msg_recvmsg(), since it only looks at MSG_PEEK? If so, maybe 
worth mentioning in the commit message.

> +
> +	if (flags & MSG_OOB)
> +		return -EOPNOTSUPP;
> +
> +	if (!len)
> +		return 0;
> +

Is the order of the checks important? We should mention it in the commit 
description (if it's important or not and why).

Thanks,
Stefano

> 	psock = sk_psock_get(sk);
> 	if (unlikely(!psock))
> 		return __vsock_recvmsg(sk, msg, len, flags);
>
> --
> 2.56.0