Stefano Garzarella
2022-Oct-31 08:43 UTC
[PATCH 2/2] vsock: fix possible infinite sleep in vsock_connectible_wait_data()
On Fri, Oct 28, 2022 at 01:56:46PM -0700, Dexuan Cui wrote:>Currently vsock_connectible_has_data() may miss a wakeup operation >between vsock_connectible_has_data() == 0 and the prepare_to_wait(). > >Fix the race by adding the process to the wait qeuue before checkings/qeuue/queue>vsock_connectible_has_data(). > >Fixes: b3f7fd54881b ("af_vsock: separate wait data loop") >Signed-off-by: Dexuan Cui <decui at microsoft.com> >--- > net/vmw_vsock/af_vsock.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >index d258fd43092e..03a6b5bc6ba7 100644 >--- a/net/vmw_vsock/af_vsock.c >+++ b/net/vmw_vsock/af_vsock.c >@@ -1905,8 +1905,11 @@ static int vsock_connectible_wait_data(struct sock *sk, > err = 0; > transport = vsk->transport; > >- while ((data = vsock_connectible_has_data(vsk)) == 0) { >+ while (1) { > prepare_to_wait(sk_sleep(sk), wait, TASK_INTERRUPTIBLE); >+ data = vsock_connectible_has_data(vsk); >+ if (data != 0) >+ break; > > if (sk->sk_err != 0 || > (sk->sk_shutdown & RCV_SHUTDOWN) || >@@ -1937,6 +1940,8 @@ static int vsock_connectible_wait_data(struct sock *sk, > err = -EAGAIN; > break; > } >+ >+ finish_wait(sk_sleep(sk), wait);Since we are going to call again prepare_to_wait() on top of the loop, is finish_wait() call here really needed? What about following what we do in vsock_accept and vsock_connect? prepare_to_wait() while (condition) { ... prepare_to_wait(); } finish_wait() I find it a little more readable, but your solution is fine too. Thanks, Stefano
Frederic Dalleau
2022-Nov-01 20:21 UTC
[PATCH 2/2] vsock: fix possible infinite sleep in vsock_connectible_wait_data()
Hi Dexan, Stephano, This solution has been proposed here, https://lists.linuxfoundation.org/pipermail/virtualization/2022-August/062656.html Best regards, Fr?d?ric On Mon, Oct 31, 2022 at 9:43 AM Stefano Garzarella <sgarzare at redhat.com> wrote:> On Fri, Oct 28, 2022 at 01:56:46PM -0700, Dexuan Cui wrote: > >Currently vsock_connectible_has_data() may miss a wakeup operation > >between vsock_connectible_has_data() == 0 and the prepare_to_wait(). > > > >Fix the race by adding the process to the wait qeuue before checking > > s/qeuue/queue > > >vsock_connectible_has_data(). > > > >Fixes: b3f7fd54881b ("af_vsock: separate wait data loop") > >Signed-off-by: Dexuan Cui <decui at microsoft.com> > >--- > > net/vmw_vsock/af_vsock.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c > >index d258fd43092e..03a6b5bc6ba7 100644 > >--- a/net/vmw_vsock/af_vsock.c > >+++ b/net/vmw_vsock/af_vsock.c > >@@ -1905,8 +1905,11 @@ static int vsock_connectible_wait_data(struct sock > *sk, > > err = 0; > > transport = vsk->transport; > > > >- while ((data = vsock_connectible_has_data(vsk)) == 0) { > >+ while (1) { > > prepare_to_wait(sk_sleep(sk), wait, TASK_INTERRUPTIBLE); > >+ data = vsock_connectible_has_data(vsk); > >+ if (data != 0) > >+ break; > > > > if (sk->sk_err != 0 || > > (sk->sk_shutdown & RCV_SHUTDOWN) || > >@@ -1937,6 +1940,8 @@ static int vsock_connectible_wait_data(struct sock > *sk, > > err = -EAGAIN; > > break; > > } > >+ > >+ finish_wait(sk_sleep(sk), wait); > > Since we are going to call again prepare_to_wait() on top of the loop, > is finish_wait() call here really needed? > > What about following what we do in vsock_accept and vsock_connect? > > prepare_to_wait() > > while (condition) { > ... > prepare_to_wait(); > } > > finish_wait() > > I find it a little more readable, but your solution is fine too. > > Thanks, > Stefano > > _______________________________________________ > Virtualization mailing list > Virtualization at lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/virtualization >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20221101/0f45bb1f/attachment.html>