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

[PATCH net v5] 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() sleeps in vsock_msg_wait_data() with the socket lock
held, so the virtio delivery worker, which needs that lock, deadlocks
with the reader and all vsock rx on that worker stops.

Release the lock around the wait. The reader can then wake without
data, so the wait loops until data arrives, the timeout, a signal
(wait_woken() returns at once while one is pending), or a state in
which __vsock_recvmsg() would not wait. Those states, and zero-length,
MSG_OOB and MSG_ERRQUEUE reads, go to __vsock_recvmsg(), as Alexei
suggested; a non-blocking read now sees them before -EAGAIN. The loop
and the zero-length check belong to the fix (without them the reader
returns early or spins with the lock held), so this is one patch;
MSG_OOB and MSG_ERRQUEUE just share that path. One tested difference
remains: a reader asleep at a virtio transport reset gets -ENOTCONN,
where a native socket gets -ECONNRESET (0 on seqpacket).

The deadlock and the MSG_ERRQUEUE case were found with the help of
Anthropic's Claude, the rest by the netdev Sashiko reviews and Alexei.
Each was reproduced in a VM without kernel changes or error injection
(the resets unbind the guest's virtio-vsock device), except one Sashiko
case that was checked in the code; with this patch they behave as on a
native socket apart from the reset case above. The vsock vmtest.sh
single-VM tests and the BPF sockmap selftests still pass, though they
never sleep in this wait.

Fixes: 634f1a7110b4 ("vsock: support sockmap")
Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Jerome Mohm <jrmmhm.kernel@eldare.de>
---
Changes in v5:
- One patch, using Alexei's approach: in the states where
  __vsock_recvmsg() does not wait, and for zero-length, MSG_OOB and
  MSG_ERRQUEUE reads, fall back to __vsock_recvmsg() instead of
  copying its return values (Alexei).
- So after a reset, recv(), a zero-length read and MSG_OOB return
  -ENOTCONN, and a peer shutdown with SO_INQ reports SCM_INQ, as on a
  socket outside a sockmap. A reset while the reader sleeps now
  returns -ENOTCONN instead of -ECONNRESET.
- This also covers the netdev Sashiko review of v4: SEQPACKET after a
  reset, MSG_OOB on a closed socket and SCM_INQ on a zero-length read;
  SHUT_RD with a pending sk_err by code reading only. A SEQPACKET
  reader asleep at a reset still gets -ENOTCONN (native 0).
- Don't cite the selftests as coverage of the wait (Alexei).
- Link to v4: https://patch.msgid.link/20261106-kbh3-1-022-fix-v2-v4-0-c7d36703814d@eldare.de

Not tested: virtio-mmio, repeated runs, a sockmap delete while a
reader sleeps, a signal arriving in the same wakeup as the timeout
(returns -EAGAIN, native -EINTR), and the vmtest.sh ns_* tests.
Still left for separate patches, as in v4: data that sits only in
psock->ingress_skb, and READ_ONCE()/WRITE_ONCE() for the lockless
accesses.

Changes in v4: comments, one data <= 0 check and message changes
(Stefano); rebase; retest with a new dataless-wakeup reproducer.
Changes in v3: split into two patches (Michal); end the wait after a
reset read with SO_ERROR, and MSG_OOB returns -EOPNOTSUPP (Sashiko).
Changes in v2: loop in the wait, len == 0 guard (Sashiko); MSG_ERRQUEUE
to the native path.
---
 net/vmw_vsock/vsock_bpf.c | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
index 9049d2648646..c0c9e69860dc 100644
--- a/net/vmw_vsock/vsock_bpf.c
+++ b/net/vmw_vsock/vsock_bpf.c
@@ -34,24 +34,35 @@ 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;
+	return sk->sk_err || sk->sk_state != TCP_ESTABLISHED ||
+	       (sk->sk_shutdown & RCV_SHUTDOWN) ||
+	       (vsock_sk(sk)->peer_shutdown & SEND_SHUTDOWN);
+}
+
+static int vsock_msg_wait_data(struct sock *sk, struct sk_psock *psock,
+			       long timeo)
+{
+	int ret = 0;
 
 	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 (!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 +91,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 +116,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);

---
base-commit: 6d2cf411aba2aa0bd641b49be5b1964ac5097a50
change-id: 20261001-kbh3-1-022-fix-v2-e3f14290641a

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

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

[netdev-bot+sashiko]

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

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

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] vsock_msg_wait_data() now drops the socket lock around
  wait_woken().
- [Low] In vsock_bpf_recvmsg(), a reader asleep during a transport reset
  gets -ENOTCONN, or 0 if SOCK_DONE is set.
- [Low] vsock_msg_wait_data() (net/vmw_vsock/vsock_bpf.c:54-66) checks
  `if (!timeo)` before `signal_pending(current)`.

--

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

This patch drops the socket lock around wait_woken() in
vsock_msg_wait_data(), so the virtio rx worker no longer deadlocks
against a sockmap reader. The wait now loops until one of these
happens: data arrives, the timeout expires, a signal is pending, or
vsock_rx_done() reports a state in which __vsock_recvmsg() would not
wait. Those states, and zero-length, MSG_OOB and MSG_ERRQUEUE reads,
are handed to __vsock_recvmsg().

> diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
> index 9049d26486460..c0c9e69860dca 100644
> --- a/net/vmw_vsock/vsock_bpf.c
> +++ b/net/vmw_vsock/vsock_bpf.c
> @@ -34,24 +34,35 @@ 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);
> +}

[Severity: Low]
Does this comment hold for the sk_err case after a transport reset?

virtio_vsock_reset_sock() sets sk_state = TCP_CLOSE and
sk_err = ECONNRESET, then calls sk_error_report(). A reader asleep in
vsock_msg_wait_data() wakes up and finds no data. vsock_rx_done() is
true, so it leaves the loop with ret = 0. vsock_bpf_recvmsg() then
sees sk_psock_queue_empty() and calls __vsock_recvmsg().

__vsock_connectible_recvmsg() checks the state first:

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;
	}
    ...
}

So the sk_err conversion in __vsock_stream_recvmsg() is never reached:

	if (sk->sk_err)
		err = -sk->sk_err;

A native reader that is already waiting in
vsock_connectible_wait_data() has passed the state check. It returns
-ECONNRESET on SOCK_STREAM and 0 on SOCK_SEQPACKET.

The commit message already notes this ("a reader asleep at a virtio
transport reset gets -ENOTCONN, where a native socket gets -ECONNRESET
(0 on seqpacket)"). It is also better than the -EAGAIN this reader got
before the patch.

The wait loop stops on sk_err, but the code it hands off to can't
report that error. Could the wait path return the pending error the
way the native path does? If not, could the comment be reworded to
cover this case?

[ ... ]

> +	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;
> +		}

[Severity: Low]
What happens when the timeout and a signal land in the same wakeup?

Say wait_woken() returns 0 because SO_RCVTIMEO ran out, and a signal
is pending at the same time. The next iteration hits the !timeo test
first and returns -EAGAIN. The signal check never runs.

The native vsock_connectible_wait_data() checks for a signal first
after sleeping:

	if (signal_pending(current)) {
		err = sock_intr_errno(timeout);
		break;
	} else if (timeout == 0) {
		err = -EAGAIN;
		break;
	}

That returns -EINTR for a finite timeout. The patch notes list this
case under "Not tested" ("returns -EAGAIN, native -EINTR").

Simply swapping the two checks at the top of the loop would make a
non-blocking read with a pending signal return -EINTR. Would checking
signal_pending() right after wait_woken() returns, as the native loop
does, make this match?

> +		release_sock(sk);
> +		timeo = wait_woken(&wait, TASK_INTERRUPTIBLE, timeo);
> +		lock_sock(sk);
>  	}

[Severity: Medium]
Now that the lock is dropped around wait_woken(), can vsk->transport
be NULL when the loop condition is checked again?

The WARN_ON_ONCE(!vsk->transport) check in vsock_bpf_recvmsg() runs
only once, at entry. After lock_sock(), the loop condition evaluates
vsock_has_data()->vsock_connectible_has_data() before
vsock_rx_done(), and that function has:

	if (WARN_ON(!vsk->transport))
		return 0;

While the reader is unlocked, vsock_connect()->vsock_assign_transport()
can release the old transport, deassign it, and then return early
without assigning a new one:

	vsk->transport->release(vsk);
	vsock_deassign_transport(vsk);
	...
	if (!new_transport->seqpacket_allow ||
	    !new_transport->seqpacket_allow(vsk, remote_cid)) {
		module_put(new_transport->module);
		return -ESOCKTNOSUPPORT;
	}
	...
	ret = new_transport->init(vsk, psk);
	if (ret) {
		module_put(new_transport->module);
		return ret;
	}

One possible sequence:

reader (sockmap socket)          connect thread
vsock_bpf_recvmsg()
  vsock_msg_wait_data()
    release_sock()
    wait_woken()
                                 transport reset: TCP_CLOSE, sk_err set
                                 earlier vsock_connect() retakes the
                                   lock, sees sk_err, sets
                                   SS_UNCONNECTED
                                 connect() again to a CID on another
                                   transport
                                   vsock_assign_transport()
                                     vsock_deassign_transport()
                                     seqpacket_allow() or init() fails
    lock_sock()
    vsock_has_data()
      vsock_connectible_has_data()
        WARN_ON(!vsk->transport)

The visible effect is a WARN splat, or a panic with panic_on_warn.
vsock_connectible_has_data() returns 0, vsock_rx_done() then returns
true, and __vsock_connectible_recvmsg() handles a NULL transport. The
native vsock_connectible_wait_data() checks in the same order.

Before this patch, the lock was held for the whole wait on the BPF
path, so this interleaving could not happen there. Would checking
vsock_rx_done() or !vsk->transport before vsock_has_data() in the loop
condition avoid the warning?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109-kbh3-1-022-fix-v2-v5-1-4d9af04f0fad%40eldare.de

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

[Stefano Garzarella]

On Mon, Nov 09, 2026 at 09:08:29PM +0100, Jerome Mohm via B4 Relay 
wrote:
>From: Jerome Mohm <jrmmhm.kernel@eldare.de>
>
>vsock_bpf_recvmsg() sleeps in vsock_msg_wait_data() with the socket lock
>held, so the virtio delivery worker, which needs that lock, deadlocks
>with the reader and all vsock rx on that worker stops.
>
>Release the lock around the wait. The reader can then wake without
>data, so the wait loops until data arrives, the timeout, a signal
>(wait_woken() returns at once while one is pending), or a state in
>which __vsock_recvmsg() would not wait. Those states, and zero-length,
>MSG_OOB and MSG_ERRQUEUE reads, go to __vsock_recvmsg(), as Alexei
>suggested; a non-blocking read now sees them before -EAGAIN. The loop
>and the zero-length check belong to the fix (without them the reader
>returns early or spins with the lock held), so this is one patch;

I don't understand why that fix can't be in a fist patch of the 
series...

>MSG_OOB and MSG_ERRQUEUE just share that path. One tested difference
>remains: a reader asleep at a virtio transport reset gets -ENOTCONN,
>where a native socket gets -ECONNRESET (0 on seqpacket).
>
>The deadlock and the MSG_ERRQUEUE case were found with the help of
>Anthropic's Claude, the rest by the netdev Sashiko reviews and Alexei.
>Each was reproduced in a VM without kernel changes or error injection
>(the resets unbind the guest's virtio-vsock device), except one Sashiko
>case that was checked in the code; with this patch they behave as on a
>native socket apart from the reset case above. The vsock vmtest.sh
>single-VM tests and the BPF sockmap selftests still pass, though they
>never sleep in this wait.
>
>Fixes: 634f1a7110b4 ("vsock: support sockmap")
>Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
>Cc: stable@vger.kernel.org
>Signed-off-by: Jerome Mohm <jrmmhm.kernel@eldare.de>
>---
>Changes in v5:
>- One patch, using Alexei's approach: in the states where
>  __vsock_recvmsg() does not wait, and for zero-length, MSG_OOB and
>  MSG_ERRQUEUE reads, fall back to __vsock_recvmsg() instead of
>  copying its return values (Alexei).
>- So after a reset, recv(), a zero-length read and MSG_OOB return
>  -ENOTCONN, and a peer shutdown with SO_INQ reports SCM_INQ, as on a
>  socket outside a sockmap. A reset while the reader sleeps now
>  returns -ENOTCONN instead of -ECONNRESET.
>- This also covers the netdev Sashiko review of v4: SEQPACKET after a
>  reset, MSG_OOB on a closed socket and SCM_INQ on a zero-length read;
>  SHUT_RD with a pending sk_err by code reading only. A SEQPACKET
>  reader asleep at a reset still gets -ENOTCONN (native 0).
>- Don't cite the selftests as coverage of the wait (Alexei).
>- Link to v4: https://patch.msgid.link/20261106-kbh3-1-022-fix-v2-v4-0-c7d36703814d@eldare.de
>
>Not tested: virtio-mmio, repeated runs, a sockmap delete while a
>reader sleeps, a signal arriving in the same wakeup as the timeout
>(returns -EAGAIN, native -EINTR), and the vmtest.sh ns_* tests.
>Still left for separate patches, as in v4: data that sits only in
>psock->ingress_skb, and READ_ONCE()/WRITE_ONCE() for the lockless
>accesses.
>
>Changes in v4: comments, one data <= 0 check and message changes
>(Stefano); rebase; retest with a new dataless-wakeup reproducer.
>Changes in v3: split into two patches (Michal); end the wait after a
>reset read with SO_ERROR, and MSG_OOB returns -EOPNOTSUPP (Sashiko).
>Changes in v2: loop in the wait, len == 0 guard (Sashiko); MSG_ERRQUEUE
>to the native path.
>---
> net/vmw_vsock/vsock_bpf.c | 43 ++++++++++++++++++++++++++++---------------
> 1 file changed, 28 insertions(+), 15 deletions(-)
>
>diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
>index 9049d2648646..c0c9e69860dc 100644
>--- a/net/vmw_vsock/vsock_bpf.c
>+++ b/net/vmw_vsock/vsock_bpf.c
>@@ -34,24 +34,35 @@ 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;
>+	return sk->sk_err || sk->sk_state != TCP_ESTABLISHED ||
>+	       (sk->sk_shutdown & RCV_SHUTDOWN) ||
>+	       (vsock_sk(sk)->peer_shutdown & SEND_SHUTDOWN);
>+}
>+
>+static int vsock_msg_wait_data(struct sock *sk, struct sk_psock *psock,
>+			       long timeo)
>+{
>+	int ret = 0;
>
> 	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 (!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 +91,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);

... I mean, why this change ^^^ can't be in patch 1/2?

BTW I'm not asking to split it (at least for now).

I saw some comments from sashiko, can you reply to them?
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109-kbh3-1-022-fix-v2-v5-1-4d9af04f0fad%40eldare.de

Thanks,
Stefano

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

[jrmmhm.kernel]

On Thu, Nov 12, 2026 at 06:23:00PM +0100, Stefano Garzarella wrote:
>On Mon, Nov 09, 2026 at 09:08:29PM +0100, Jerome Mohm via B4 Relay
>wrote:
>>The loop
>>and the zero-length check belong to the fix (without them the reader
>>returns early or spins with the lock held), so this is one patch;
>
>I don't understand why that fix can't be in a fist patch of the
>series...
>
>>+    if (unlikely(!len || (flags & (MSG_OOB | MSG_ERRQUEUE))))
>>+        return __vsock_recvmsg(sk, msg, len, flags);
>
>... I mean, why this change ^^^ can't be in patch 1/2?
>
>BTW I'm not asking to split it (at least for now).

It can: the loop is needed for the deadlock fix, since without it the
reader returns -EAGAIN early when it wakes without data, but the early
return isn't. A zero-length read with data in the psock queue already
spins with the lock held on net, and as far as I can tell from the code
the early return alone fixes that, plus the MSG_OOB and MSG_ERRQUEUE
cases. In v4 those reads were patch 2/2. They went into one patch only
because Alexei's diff replaced both, and my v5 sentence justified that
badly. You said you're not asking for a split, but since the early
return isn't needed for the deadlock, it would fit as its own patch:
should v6 have it as 1/2 and the lock change as 2/2, or stay one patch?

>I saw some comments from sashiko, can you reply to them?

I'll wait for your answer before sending v6, and I'm replying to the
Sashiko comments separately.

Thanks,
Jerome

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

[jrmmhm.kernel]

On Thu, Nov 12, 2026 at 08:08:52AM +0000, netdev-bot+sashiko@kernel.org wrote:
>> -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)
>
>[Severity: Low]
>Does this comment hold for the sk_err case after a transport reset?
[ ... ]
>The wait loop stops on sk_err, but the code it hands off to can't
>report that error. Could the wait path return the pending error the
>way the native path does? If not, could the comment be reworded to
>cover this case?

I reworded the comment and kept -ENOTCONN, which the commit
message already notes. Returning the pending error from the wait would
bring back the copy of the native return values that Alexei asked me
to drop in v4.

>> +    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;
>> +        }
>
>[Severity: Low]
>What happens when the timeout and a signal land in the same wakeup?
[ ... ]
>Would checking signal_pending() right after wait_woken() returns, as
>the native loop does, make this match?

I'm leaving this one as is: it only changes which error a reader gets
when the timeout and a signal land in the same wakeup, and I don't
think that is worth another change to the loop in this patch.

>> +        release_sock(sk);
>> +        timeo = wait_woken(&wait, TASK_INTERRUPTIBLE, timeo);
>> +        lock_sock(sk);
>>      }
>
>[Severity: Medium]
>Now that the lock is dropped around wait_woken(), can vsk->transport
>be NULL when the loop condition is checked again?
[ ... ]
>Before this patch, the lock was held for the whole wait on the BPF
>path, so this interleaving could not happen there. Would checking
>vsock_rx_done() or !vsk->transport before vsock_has_data() in the loop
>condition avoid the warning?

I haven't reproduced this, but the code agrees with it: v6 will check
vsk->transport first in the wait loop, so a reader woken after its
transport is gone no longer calls vsock_has_data(). The entry
WARN_ON_ONCE() in vsock_bpf_recvmsg() stays as it is, so a later recv()
on such a socket still reaches it, as it does on net today.

pw-bot: cr

Jerome

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

[Stefano Garzarella]

On Thu, Nov 12, 2026 at 07:47:11PM +0000, jrmmhm.kernel@eldare.de wrote:
>On Thu, Nov 12, 2026 at 06:23:00PM +0100, Stefano Garzarella wrote:
>>On Mon, Nov 09, 2026 at 09:08:29PM +0100, Jerome Mohm via B4 Relay
>>wrote:
>>>The loop
>>>and the zero-length check belong to the fix (without them the reader
>>>returns early or spins with the lock held), so this is one patch;
>>
>>I don't understand why that fix can't be in a fist patch of the
>>series...
>>
>>>+    if (unlikely(!len || (flags & (MSG_OOB | MSG_ERRQUEUE))))
>>>+        return __vsock_recvmsg(sk, msg, len, flags);
>>
>>... I mean, why this change ^^^ can't be in patch 1/2?
>>
>>BTW I'm not asking to split it (at least for now).
>
>It can: the loop is needed for the deadlock fix, since without it the
>reader returns -EAGAIN early when it wakes without data, but the early
>return isn't. A zero-length read with data in the psock queue already
>spins with the lock held on net, and as far as I can tell from the code
>the early return alone fixes that, plus the MSG_OOB and MSG_ERRQUEUE
>cases. In v4 those reads were patch 2/2. They went into one patch only
>because Alexei's diff replaced both, and my v5 sentence justified that
>badly. You said you're not asking for a split, but since the early
>return isn't needed for the deadlock, it would fit as its own patch:
>should v6 have it as 1/2 and the lock change as 2/2, or stay one patch?

Yeah, I'd prefer 2 patches with this change as first patch.

>
>>I saw some comments from sashiko, can you reply to them?
>
>I'll wait for your answer before sending v6, and I'm replying to the
>Sashiko comments separately.

Great, thanks for that!

Stefano