[PATCH net v4 0/2] vsock/bpf: fix waiting and flag handling in recvmsg

[PATCH net v4 0/2] vsock/bpf: fix waiting and flag handling in recvmsg

[Jerome Mohm via B4 Relay]

vsock_bpf_recvmsg() holds lock_sock() while it sleeps for data, so the
transport's delivery worker, which needs that lock to queue the data,
deadlocks with the reader. Patch 1 releases the lock around the wait and
makes the wait loop with the exit conditions of the native path. Patch 2
handles the reads that should never reach the sockmap wait: len == 0,
MSG_ERRQUEUE and MSG_OOB.

Not addressed here, as in v3: data that sits only in psock->ingress_skb,
and READ_ONCE()/WRITE_ONCE() for the lockless accesses; both are left for
separate patches. Not run: the ns_* tests of vmtest.sh (they need root on
the host); the reproducers ran in a guest over virtio-vsock (PCI) and
vsock-loopback only.

The deadlock and the MSG_ERRQUEUE case were found with the help of
Anthropic's Claude; the other cases come from the netdev Sashiko review.
The reproducers are available on request.

Changes in v4:
- 1/2: comment why the wait stops on sk_err and why it checks
  signal_pending() (Stefano).
- 1/2: one data <= 0 check in vsock_bpf_recvmsg() (Stefano).
- 1/2: say why this is one patch and how non-blocking reads change
  (Stefano).
- 2/2: say that MSG_ERRQUEUE consumed stream bytes and why the checks
  come in this order (Stefano).
- Shorter commit messages.
- Rebase onto current net.
- Retested, with a new reproducer for a dataless wakeup during a timed
  wait.
- Link to v3: https://patch.msgid.link/20261028-kbh3-1-022-fix-v2-v3-0-bc0752b2d3db@eldare.de

Changes in v3:
- Split into two patches (Michal): 1/2 releases the lock and loops in
  the wait, 2/2 handles len == 0, MSG_ERRQUEUE and MSG_OOB.
- 1/2: also end the wait once the socket is no longer TCP_ESTABLISHED
  (0 with SOCK_DONE, else -ENOTCONN, as __vsock_connectible_recvmsg()
  does), so a reset connection whose error was read with SO_ERROR no
  longer waits out the timeout (Sashiko review of v2).
- 2/2: reject MSG_OOB with -EOPNOTSUPP before the len == 0 check, as
  unix_bpf does; this also stops MSG_OOB from consuming ordinary bytes
  (Sashiko review of v2).
- Stefano's Acked-by on v1 is not carried: neither patch matches v1.
- Drop the Assisted-by trailer (removed upstream); tool use is disclosed
  above instead.
- Rebase onto current net.
- Link to v2: https://lore.kernel.org/netdev/20261001-kbh3-1-022-fix-v2-v1-1-cfae958a1976@eldare.de/

Changes in v2:
- Make vsock_msg_wait_data() loop, check sk_err and the peer
  SEND_SHUTDOWN, and add the len == 0 guard (Sashiko review of v1);
  route MSG_ERRQUEUE to the native path ahead of the guard.
- Link to v1: https://lore.kernel.org/netdev/20260929-kbh3-1-022-fix-v1-1-cc97cc95d269@eldare.de/

---
Jerome Mohm (2):
      vsock/bpf: release sock lock while waiting for data in recvmsg
      vsock/bpf: fix zero-length, MSG_OOB and MSG_ERRQUEUE reads in recvmsg

 net/vmw_vsock/vsock_bpf.c | 77 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 63 insertions(+), 14 deletions(-)
---
base-commit: 6d2cf411aba2aa0bd641b49be5b1964ac5097a50
change-id: 20261001-kbh3-1-022-fix-v2-e3f14290641a

Best regards,
--  
Jerome Mohm <jrmmhm.kernel@eldare.de>

[PATCH net v4 1/2] vsock/bpf: release sock lock while waiting for data in recvmsg

[Jerome Mohm via B4 Relay]

From: Jerome Mohm <jrmmhm.kernel@eldare.de>

vsock_bpf_recvmsg() holds lock_sock() while vsock_msg_wait_data() sleeps
in wait_woken(). The transport delivers data in
virtio_transport_recv_pkt(), which takes the same lock, so the reader
and the delivery worker deadlock: recv() hangs until a signal or
SO_RCVTIMEO, and all vsock rx handled by that worker stops.

Release the lock around wait_woken(), as unix_bpf and tcp_bpf release
theirs. That alone ends the deadlock but makes a blocking recv()
return -EAGAIN early: the reader is also woken without data, for
example by a credit update. So the wait also loops until data arrives
or one of the native exits applies: a socket error, a local or peer
shutdown, a socket that is no longer connected, the timeout or a
pending signal. The loop alone, with the lock still held, keeps the
deadlock, so the fix needs both halves and stays one patch.

For a non-blocking read, a socket error, a peer shutdown and a socket
that is not connected are now reported before -EAGAIN, as on the native
path. A blocking read interrupted by a signal returns sock_intr_errno()
instead of -EAGAIN. A recv() that starts after a virtio transport reset
returns -ECONNRESET until SO_ERROR has read the error, where the native
path returns -ENOTCONN.

The deadlock was found with the help of Anthropic's Claude; Sashiko
reviews of two earlier sockmap patches had already noted it as a
pre-existing issue. The other cases were reported by the netdev Sashiko
review. Each was reproduced in a VM with a vsock socket in a sockmap,
without kernel changes or error injection; the reset case unbinds the
guest's virtio-vsock device. With the series applied they behave as on
a native socket, except for the reset case above, and the vsock
vmtest.sh single-VM tests and the BPF sockmap test_progs pass.

Fixes: 634f1a7110b4 ("vsock: support sockmap")
Cc: stable@vger.kernel.org
Signed-off-by: Jerome Mohm <jrmmhm.kernel@eldare.de>
---
 net/vmw_vsock/vsock_bpf.c | 68 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 54 insertions(+), 14 deletions(-)

diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
index 9049d2648646..6d96ff03f28c 100644
--- a/net/vmw_vsock/vsock_bpf.c
+++ b/net/vmw_vsock/vsock_bpf.c
@@ -34,24 +34,63 @@ static bool vsock_has_data(struct sock *sk, struct sk_psock *psock)
 	return vsock_sk_has_data(sk, psock);
 }
 
-static bool vsock_msg_wait_data(struct sock *sk, struct sk_psock *psock, long timeo)
+/* Returns 1 if data is ready, 0 on EOF/shutdown, or a negative error. */
+static int vsock_msg_wait_data(struct sock *sk, struct sk_psock *psock, long timeo)
 {
-	bool ret;
+	struct vsock_sock *vsk = vsock_sk(sk);
+	int ret;
 
 	DEFINE_WAIT_FUNC(wait, woken_wake_function);
 
-	if (sk->sk_shutdown & RCV_SHUTDOWN)
-		return true;
-
-	if (!timeo)
-		return false;
-
 	add_wait_queue(sk_sleep(sk), &wait);
 	sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
-	ret = vsock_has_data(sk, psock);
-	if (!ret) {
-		wait_woken(&wait, TASK_INTERRUPTIBLE, timeo);
-		ret = vsock_has_data(sk, psock);
+	while (1) {
+		if (vsock_has_data(sk, psock)) {
+			ret = 1;
+			break;
+		}
+
+		/* Stop on a socket error, as vsock_connectible_wait_data()
+		 * does, so that recv() returns it; the checks below would
+		 * end the wait with 0 or -ENOTCONN instead.
+		 */
+		if (sk->sk_err) {
+			ret = -sk->sk_err;
+			break;
+		}
+
+		if ((sk->sk_shutdown & RCV_SHUTDOWN) ||
+		    (vsk->peer_shutdown & SEND_SHUTDOWN)) {
+			ret = 0;
+			break;
+		}
+
+		/* virtio_vsock_reset_sock() sets no shutdown bits, and
+		 * SO_ERROR clears sk_err: check the state the way
+		 * __vsock_connectible_recvmsg() does.
+		 */
+		if (sk->sk_state != TCP_ESTABLISHED) {
+			ret = sock_flag(sk, SOCK_DONE) ? 0 : -ENOTCONN;
+			break;
+		}
+
+		if (!timeo) {
+			ret = -EAGAIN;
+			break;
+		}
+
+		release_sock(sk);
+		timeo = wait_woken(&wait, TASK_INTERRUPTIBLE, timeo);
+		lock_sock(sk);
+
+		/* With a signal pending, wait_woken() returns at once, so
+		 * without this check the loop would spin until the timeout,
+		 * or forever without one.
+		 */
+		if (signal_pending(current)) {
+			ret = sock_intr_errno(timeo);
+			break;
+		}
 	}
 	sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
 	remove_wait_queue(sk_sleep(sk), &wait);
@@ -101,9 +140,10 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
 	copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
 	while (copied == 0) {
 		long timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
+		int data = vsock_msg_wait_data(sk, psock, timeo);
 
-		if (!vsock_msg_wait_data(sk, psock, timeo)) {
-			copied = -EAGAIN;
+		if (data <= 0) {
+			copied = data;
 			break;
 		}
 

-- 
2.56.0

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

[Jerome Mohm via B4 Relay]

From: Jerome Mohm <jrmmhm.kernel@eldare.de>

vsock_bpf_recvmsg() sends every read that the transport queue cannot
serve through sk_msg_recvmsg() and the sockmap wait, which handle
neither a zero length nor these flags:

 - with len == 0 and data in the psock ingress queue the loop spins with
   lock_sock() held, and the delivery worker blocks on the lock;
 - MSG_ERRQUEUE with data in the psock ingress queue consumes stream
   bytes, and a zero-length MSG_ERRQUEUE read, as MSG_ZEROCOPY users
   do, never reaches the error queue: it gets -EAGAIN, or waits for
   stream data if it blocks;
 - MSG_OOB consumes ordinary stream bytes or waits for data, where the
   native path returns -EOPNOTSUPP.

Handle them at the top of vsock_bpf_recvmsg(). The order matters:
MSG_ERRQUEUE goes to the native path first, as in tcp_bpf, or a
zero-length error-queue read would return 0; MSG_OOB comes before
len == 0, as in unix_bpf, so recv(fd, buf, 0, MSG_OOB) returns
-EOPNOTSUPP as on a connected native socket.

The MSG_ERRQUEUE case was found with the help of Anthropic's Claude, the
zero-length and MSG_OOB cases by the netdev Sashiko review. Each was
reproduced in a VM with a vsock-loopback socket in a sockmap, without
kernel changes or error injection. With the series applied the reads
behave as on a connected native socket, and the vsock vmtest.sh
single-VM tests and the BPF sockmap test_progs pass.

Fixes: 634f1a7110b4 ("vsock: support sockmap")
Cc: stable@vger.kernel.org
Signed-off-by: Jerome Mohm <jrmmhm.kernel@eldare.de>
---
 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 6d96ff03f28c..58b32fbc1f4b 100644
--- a/net/vmw_vsock/vsock_bpf.c
+++ b/net/vmw_vsock/vsock_bpf.c
@@ -119,6 +119,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 (flags & MSG_OOB)
+		return -EOPNOTSUPP;
+
+	if (!len)
+		return 0;
+
 	psock = sk_psock_get(sk);
 	if (unlikely(!psock))
 		return __vsock_recvmsg(sk, msg, len, flags);

-- 
2.56.0

Re: [PATCH net v4 1/2] vsock/bpf: release sock lock while waiting for data in recvmsg

[Alexei Starovoitov]

On Fri, Nov 06, 2026 at 01:57 PM Jerome Mohm via B4 Relay <devnull+jrmmhm.kernel.eldare.de@kernel.org> wrote:
> example by a credit update. So the wait also loops until data arrives
> or one of the native exits applies: a socket error, a local or peer
> shutdown, a socket that is no longer connected, the timeout or a
> pending signal. The loop alone, with the lock still held, keeps the
> deadlock, so the fix needs both halves and stays one patch.

No need for a 2nd copy of the return values.
The old code returns true on RCV_SHUTDOWN. The psock queue is empty, so
the caller goes to __vsock_recvmsg(), which picks the return value.
tcp_bpf, udp_bpf and unix_bpf do the same.
Do it for sk_err, peer_shutdown and sk_state too.
__vsock_recvmsg() doesn't wait in any of these states.
It handles len == 0, MSG_OOB and MSG_ERRQUEUE as well.

The copy differs from a socket that is not in a sockmap in four cases.
The diff differs in one, the last row:

  recv()                          no sockmap    this series   the diff
  ------------------------------  ------------  ------------  -----------
  after a virtio reset            -ENOTCONN     -ECONNRESET   -ENOTCONN
  after a reset, len == 0         -ENOTCONN     0             -ENOTCONN
  after a reset, MSG_OOB          -ENOTCONN     -EOPNOTSUPP   -ENOTCONN
  after peer shutdown, SO_INQ on  0 + SCM_INQ   0             0 + SCM_INQ
  asleep when the reset comes     -ECONNRESET   -ECONNRESET   -ENOTCONN

Instead of patches 1 and 2 (compile tested only):

diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
index 9049d26..3cf017d 100644
--- a/net/vmw_vsock/vsock_bpf.c
+++ b/net/vmw_vsock/vsock_bpf.c
@@ -34,24 +34,34 @@ static bool vsock_has_data(struct sock *sk, struct sk_psock *psock)
 	return vsock_sk_has_data(sk, psock);
 }

-static bool vsock_msg_wait_data(struct sock *sk, struct sk_psock *psock, long timeo)
+/* __vsock_recvmsg() doesn't wait in these states and knows what to return. */
+static bool vsock_rx_done(struct sock *sk)
 {
-	bool ret;
-
-	DEFINE_WAIT_FUNC(wait, woken_wake_function);
+	return sk->sk_err || sk->sk_state != TCP_ESTABLISHED ||
+	       (sk->sk_shutdown & RCV_SHUTDOWN) ||
+	       (vsock_sk(sk)->peer_shutdown & SEND_SHUTDOWN);
+}

-	if (sk->sk_shutdown & RCV_SHUTDOWN)
-		return true;
+static int vsock_msg_wait_data(struct sock *sk, struct sk_psock *psock, long timeo)
+{
+	int ret = 0;

-	if (!timeo)
-		return false;
+	DEFINE_WAIT_FUNC(wait, woken_wake_function);

 	add_wait_queue(sk_sleep(sk), &wait);
 	sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
-	ret = vsock_has_data(sk, psock);
-	if (!ret) {
-		wait_woken(&wait, TASK_INTERRUPTIBLE, timeo);
-		ret = vsock_has_data(sk, psock);
+	while (!vsock_has_data(sk, psock) && !vsock_rx_done(sk)) {
+		if (!timeo) {
+			ret = -EAGAIN;
+			break;
+		}
+		if (signal_pending(current)) {
+			ret = sock_intr_errno(timeo);
+			break;
+		}
+		release_sock(sk);
+		timeo = wait_woken(&wait, TASK_INTERRUPTIBLE, timeo);
+		lock_sock(sk);
 	}
 	sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
 	remove_wait_queue(sk_sleep(sk), &wait);
@@ -80,6 +90,9 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
 	struct vsock_sock *vsk;
 	int copied;

+	if (unlikely(!len || (flags & (MSG_OOB | MSG_ERRQUEUE))))
+		return __vsock_recvmsg(sk, msg, len, flags);
+
 	psock = sk_psock_get(sk);
 	if (unlikely(!psock))
 		return __vsock_recvmsg(sk, msg, len, flags);
@@ -102,10 +115,9 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
 	while (copied == 0) {
 		long timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);

-		if (!vsock_msg_wait_data(sk, psock, timeo)) {
-			copied = -EAGAIN;
+		copied = vsock_msg_wait_data(sk, psock, timeo);
+		if (copied)
 			break;
-		}

 		if (sk_psock_queue_empty(psock)) {
 			release_sock(sk);

[...]

> guest's virtio-vsock device. With the series applied they behave as on
> a native socket, except for the reset case above, and the vsock
> vmtest.sh single-VM tests and the BPF sockmap test_progs pass.

None of these tests sleeps in vsock_msg_wait_data().
The sockmap tests call select() before recv() or pass MSG_DONTWAIT.
vmtest.sh doesn't use sockmap.

pw-bot: cr

Re: [PATCH net v4 1/2] vsock/bpf: release sock lock while waiting for data in recvmsg

[jrmmhm.kernel]

On Sat, 07 Nov 2026 02:09:17 +0000, Alexei Starovoitov wrote:
> No need for a 2nd copy of the return values.
[...]
>   recv()                          no sockmap    this series   the diff
>   ------------------------------  ------------  ------------  -----------
>   after a virtio reset            -ENOTCONN     -ECONNRESET   -ENOTCONN
>   after a reset, len == 0         -ENOTCONN     0             -ENOTCONN
>   after a reset, MSG_OOB          -ENOTCONN     -EOPNOTSUPP   -ENOTCONN
>   after peer shutdown, SO_INQ on  0 + SCM_INQ   0             0 + SCM_INQ
>   asleep when the reset comes     -ECONNRESET   -ECONNRESET   -ENOTCONN

Agreed. I ran your diff in a guest with the reproducers of this series:
the deadlock, the dataless wakeup, signal, timeout, peer shutdown,
MSG_ERRQUEUE and MSG_OOB cases behave as with v4, with no KASAN, lockdep
or hung task report. In the first four rows of your table the sockmap
socket now returns exactly what a native one returns.

The last row is confirmed as well. I'd accept that and say so in the
commit message, since it only changes which error a reader of an
already reset connection sees.

> None of these tests sleeps in vsock_msg_wait_data().
> The sockmap tests call select() before recv() or pass MSG_DONTWAIT.
> vmtest.sh doesn't use sockmap.

Right. v5 won't cite them as coverage of this change.

v5 will be a single patch based on your diff, with your Suggested-by.

Thanks,
Jerome

Re: [PATCH net v4 1/2] vsock/bpf: release sock lock while waiting for data in recvmsg

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message says that with the series applied the
  reproduced cases 'behave as on a native socket, except for the reset…

--

vsock/bpf: release sock lock while waiting for data in recvmsg

This patch fixes a deadlock in vsock_bpf_recvmsg(). The function held
lock_sock() while it slept in wait_woken(), and the transport needs that
lock to deliver data. The lock is now released around wait_woken(), and
the wait loops until data arrives or one of the native exit conditions
applies (socket error, shutdown, not connected, timeout or signal).

> diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
> index 9049d26486460..6d96ff03f28c4 100644
> --- a/net/vmw_vsock/vsock_bpf.c
> +++ b/net/vmw_vsock/vsock_bpf.c

[ ... ]

> +	while (1) {
> +		if (vsock_has_data(sk, psock)) {
> +			ret = 1;
> +			break;
> +		}
> +
> +		/* Stop on a socket error, as vsock_connectible_wait_data()
> +		 * does, so that recv() returns it; the checks below would
> +		 * end the wait with 0 or -ENOTCONN instead.
> +		 */
> +		if (sk->sk_err) {
> +			ret = -sk->sk_err;
> +			break;
> +		}

[Severity: Medium]
Does this match the native path for SOCK_SEQPACKET as well as
SOCK_STREAM?

vsock_connectible_wait_data() stops on sk_err, but it returns 0 rather
than the error:

net/vmw_vsock/af_vsock.c:vsock_connectible_wait_data() {
    ...
		if (sk->sk_err != 0 ||
		    (sk->sk_shutdown & RCV_SHUTDOWN) ||
		    (vsk->peer_shutdown & SEND_SHUTDOWN)) {
			break;
		}
    ...
}

Only __vsock_stream_recvmsg() converts that into -sk->sk_err afterwards.
__vsock_seqpacket_recvmsg() returns the 0 directly:

	err = vsock_connectible_wait_data(sk, &wait, timeout, NULL, 0);
	if (err <= 0)
		goto out;

With this patch, vsock_bpf_recvmsg() passes the return value through
unchanged (copied = data). sock_map accepts vsock SOCK_SEQPACKET sockets.

Take a SEQPACKET socket in a sockmap with a reader blocked in recv(). If
virtio_vsock_reset_sock() runs and sets sk_err = ECONNRESET, the reader
gets -ECONNRESET here, while a native socket returns 0. The same applies
to other sk_err values, such as EPIPE from sk_psock_report_error().

The check order also differs from __vsock_connectible_recvmsg(). That
function checks the state and transport first, then returns 0 on a local
RCV_SHUTDOWN before it waits:

	if (sk->sk_shutdown & RCV_SHUTDOWN) {
		err = 0;
		goto out;
	}

This loop checks sk_err before RCV_SHUTDOWN. So a socket that is still
TCP_ESTABLISHED, has done a local SHUT_RD and has a pending sk_err would
return -sk_err from the BPF path but 0 natively.

The commit message says:

    With the series applied they behave as on
    a native socket, except for the reset case above

and the code comment claims parity with vsock_connectible_wait_data().
Neither holds for SEQPACKET, or for the SHUT_RD plus sk_err case.

Returning the error may be the better behaviour. If so, could the commit
message and comment say so? The alternative is to follow the native
semantics for each socket type. The later patch in the series ("vsock/bpf:
fix zero-length, MSG_OOB and MSG_ERRQUEUE reads in recvmsg") does not
change vsock_msg_wait_data(), so the difference is still there at the end
of the series.

> +
> +		if ((sk->sk_shutdown & RCV_SHUTDOWN) ||
> +		    (vsk->peer_shutdown & SEND_SHUTDOWN)) {
> +			ret = 0;
> +			break;
> +		}

[ ... ]

> @@ -101,9 +140,10 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
>  	copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
>  	while (copied == 0) {
>  		long timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> +		int data = vsock_msg_wait_data(sk, psock, timeo);
>  
> -		if (!vsock_msg_wait_data(sk, psock, timeo)) {
> -			copied = -EAGAIN;
> +		if (data <= 0) {
> +			copied = data;
>  			break;
>  		}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106-kbh3-1-022-fix-v2-v4-0-c7d36703814d%40eldare.de

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new early returns in vsock_bpf_recvmsg()
  (net/vmw_vsock/vsock_bpf.c:125-129) skip the transport and sk_state…
- [Low] The new `if (!len) return 0;` in vsock_bpf_recvmsg()
  (net/vmw_vsock/vsock_bpf.c:128-129) returns before the native stream…

--

vsock/bpf: fix zero-length, MSG_OOB and MSG_ERRQUEUE reads in recvmsg

This commit adds checks at the top of vsock_bpf_recvmsg() for three cases.
MSG_ERRQUEUE goes to the native path, MSG_OOB returns -EOPNOTSUPP, and
zero-length reads return 0. Before this, these reads went through
sk_msg_recvmsg() and the sockmap wait, which could spin with the socket
lock held or consume stream bytes.

> diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
> index 6d96ff03f28c4..58b32fbc1f4b2 100644
> --- a/net/vmw_vsock/vsock_bpf.c
> +++ b/net/vmw_vsock/vsock_bpf.c
> @@ -119,6 +119,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 (flags & MSG_OOB)
> +		return -EOPNOTSUPP;
> +
> +	if (!len)
> +		return 0;

[Severity: Medium]
Do these two early returns give the right result on a sockmap'd socket that
is no longer established?

The native __vsock_connectible_recvmsg() checks the transport and socket
state before it looks at MSG_OOB or len:

net/vmw_vsock/af_vsock.c:__vsock_connectible_recvmsg() {
    ...
	if (!transport || sk->sk_state != TCP_ESTABLISHED) {
		...
		if (sock_flag(sk, SOCK_DONE))
			err = 0;
		else
			err = -ENOTCONN;

		goto out;
	}

	if (flags & MSG_OOB) {
		err = -EOPNOTSUPP;
		goto out;
	}
    ...
	if (!len) {
		err = 0;
		goto out;
	}
    ...
}

sock_map_sk_state_allowed() requires TCP_ESTABLISHED only when a vsock
STREAM or SEQPACKET socket is inserted. After that, the state can change
while the socket stays in the map. One example is a transport reset:

virtio_vsock_event_handle()
  vsock_for_each_connected_socket()
    virtio_vsock_reset_sock()
      sk->sk_state = TCP_CLOSE;
      sk->sk_err = ECONNRESET;

Another is a peer close through virtio_transport_do_close(), which sets
SOCK_DONE and moves the socket to TCP_CLOSING.

With this patch, the BPF path then returns:

- recv(fd, buf, n, MSG_OOB) after a transport reset: -EOPNOTSUPP, where
  the native path returns -ENOTCONN.
- recv(fd, NULL, 0, 0) after a transport reset: 0, where the native path
  returns -ENOTCONN.
- recv(fd, buf, n, MSG_OOB) after a peer close: -EOPNOTSUPP, where the
  native path returns 0.

The zero-length case also looks like a regression from the behaviour
before this patch. With empty queues, the old code reached
vsock_msg_wait_data(), which returned -ECONNRESET from sk_err. If SO_ERROR
had already cleared sk_err, it returned -ENOTCONN from the state check:

	/* virtio_vsock_reset_sock() sets no shutdown bits, and
	 * SO_ERROR clears sk_err: check the state the way
	 * __vsock_connectible_recvmsg() does.
	 */
	if (sk->sk_state != TCP_ESTABLISHED) {
		ret = sock_flag(sk, SOCK_DONE) ? 0 : -ENOTCONN;

The new early return now reports a successful 0 instead of that error.

unix_bpf can use these returns because its native path has no state check
before them. The vsock native path does have one, and
__vsock_connectible_recvmsg() already handles MSG_OOB and !len in the
right order without touching the psock queues.

Would it work to send both cases to __vsock_recvmsg(), the way MSG_ERRQUEUE
is already handled here?

[Severity: Low]
Does the !len early return also drop the SCM_INQ cmsg and msg_inq hint for
zero-length stream reads?

In the native path, the !len case jumps to out:, which still reports the
hint for SOCK_STREAM:

net/vmw_vsock/af_vsock.c:__vsock_connectible_recvmsg() {
    ...
out:
	if ((vsk->recvmsg_inq || msg->msg_get_inq) && err >= 0 &&
	    sk->sk_type == SOCK_STREAM) {
		msg->msg_inq = vsock_stream_inq_hint(sk);
		...
		do_cmsg = vsk->recvmsg_inq && msg->msg_inq >= 0;
	}
    ...
}

Before this patch, take a zero-length read with SO_INQ set, an empty psock
queue and data in the transport. It matched vsock_has_data() &&
sk_psock_queue_empty() and went to __vsock_recvmsg(), so it got SCM_INQ
and msg_inq. Now it returns 0 with neither.

io_uring sets msg_inq to -1 before most of its recv calls, so there the
hint is missing rather than wrong. Sending !len to __vsock_recvmsg(), as
suggested above, would fix this too.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261106-kbh3-1-022-fix-v2-v4-0-c7d36703814d%40eldare.de