mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v4 0/2] vsock/bpf: fix waiting and flag handling in recvmsg
@ 2026-11-06 21:57 Jerome Mohm via B4 Relay
  2026-11-06 21:57 ` [PATCH net v4 1/2] vsock/bpf: release sock lock while waiting for data " Jerome Mohm via B4 Relay
  2026-11-06 21:57 ` [PATCH net v4 2/2] vsock/bpf: fix zero-length, MSG_OOB and MSG_ERRQUEUE reads " Jerome Mohm via B4 Relay
  0 siblings, 2 replies; 4+ messages in thread
From: Jerome Mohm via B4 Relay @ 2026-11-06 21:57 UTC (permalink / raw)
  To: Stefano Garzarella, Bobby Eshleman, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Michael S. Tsirkin
  Cc: Michal Luczaj, virtualization, netdev, linux-kernel, bpf, stable,
	Jerome Mohm

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>



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH net v4 1/2] vsock/bpf: release sock lock while waiting for data in recvmsg
  2026-11-06 21:57 [PATCH net v4 0/2] vsock/bpf: fix waiting and flag handling in recvmsg Jerome Mohm via B4 Relay
@ 2026-11-06 21:57 ` Jerome Mohm via B4 Relay
  2026-11-07  2:09   ` Alexei Starovoitov
  2026-11-06 21:57 ` [PATCH net v4 2/2] vsock/bpf: fix zero-length, MSG_OOB and MSG_ERRQUEUE reads " Jerome Mohm via B4 Relay
  1 sibling, 1 reply; 4+ messages in thread
From: Jerome Mohm via B4 Relay @ 2026-11-06 21:57 UTC (permalink / raw)
  To: Stefano Garzarella, Bobby Eshleman, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Michael S. Tsirkin
  Cc: Michal Luczaj, virtualization, netdev, linux-kernel, bpf, stable,
	Jerome Mohm

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



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH net v4 2/2] vsock/bpf: fix zero-length, MSG_OOB and MSG_ERRQUEUE reads in recvmsg
  2026-11-06 21:57 [PATCH net v4 0/2] vsock/bpf: fix waiting and flag handling in recvmsg Jerome Mohm via B4 Relay
  2026-11-06 21:57 ` [PATCH net v4 1/2] vsock/bpf: release sock lock while waiting for data " Jerome Mohm via B4 Relay
@ 2026-11-06 21:57 ` Jerome Mohm via B4 Relay
  1 sibling, 0 replies; 4+ messages in thread
From: Jerome Mohm via B4 Relay @ 2026-11-06 21:57 UTC (permalink / raw)
  To: Stefano Garzarella, Bobby Eshleman, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Michael S. Tsirkin
  Cc: Michal Luczaj, virtualization, netdev, linux-kernel, bpf, stable,
	Jerome Mohm

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



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net v4 1/2] vsock/bpf: release sock lock while waiting for data in recvmsg
  2026-11-06 21:57 ` [PATCH net v4 1/2] vsock/bpf: release sock lock while waiting for data " Jerome Mohm via B4 Relay
@ 2026-11-07  2:09   ` Alexei Starovoitov
  0 siblings, 0 replies; 4+ messages in thread
From: Alexei Starovoitov @ 2026-11-07  2:09 UTC (permalink / raw)
  To: jrmmhm.kernel, Stefano Garzarella, Bobby Eshleman,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Michael S. Tsirkin
  Cc: Michal Luczaj, virtualization, netdev, linux-kernel, bpf, stable

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-07  2:09 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-06 21:57 [PATCH net v4 0/2] vsock/bpf: fix waiting and flag handling in recvmsg Jerome Mohm via B4 Relay
2026-11-06 21:57 ` [PATCH net v4 1/2] vsock/bpf: release sock lock while waiting for data " Jerome Mohm via B4 Relay
2026-11-07  2:09   ` Alexei Starovoitov
2026-11-06 21:57 ` [PATCH net v4 2/2] vsock/bpf: fix zero-length, MSG_OOB and MSG_ERRQUEUE reads " Jerome Mohm via B4 Relay

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®