From: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>
To: <jrmmhm.kernel@eldare.de>,
"Stefano Garzarella" <sgarzare@redhat.com>,
"Bobby Eshleman" <bobbyeshleman@meta.com>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@kernel.org>,
"Jakub Kicinski" <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Simon Horman" <horms@kernel.org>,
"Michael S. Tsirkin" <mst@redhat.com>
Cc: "Michal Luczaj" <mhal@rbox.co>, <virtualization@lists.linux.dev>,
<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<bpf@vger.kernel.org>, <stable@vger.kernel.org>
Subject: Re: [PATCH net v4 1/2] vsock/bpf: release sock lock while waiting for data in recvmsg
Date: Sat, 07 Nov 2026 02:09:17 +0000 [thread overview]
Message-ID: <DMOM2SFCFHYK.3BT44IQ90X9UB@gmail.com> (raw)
In-Reply-To: <20261106-kbh3-1-022-fix-v2-v4-1-c7d36703814d@eldare.de>
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
next prev parent reply other threads:[~2026-11-07 2:09 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-06 21:57 [PATCH net v4 0/2] vsock/bpf: fix waiting and flag handling " 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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DMOM2SFCFHYK.3BT44IQ90X9UB@gmail.com \
--to=alexei.starovoitov@gmail.com \
--cc=bobbyeshleman@meta.com \
--cc=bpf@vger.kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@kernel.org \
--cc=horms@kernel.org \
--cc=jrmmhm.kernel@eldare.de \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mhal@rbox.co \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sgarzare@redhat.com \
--cc=stable@vger.kernel.org \
--cc=virtualization@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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®