* [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
* 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
* [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
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®