lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <SA1PR21MB1335A7C6DD342BCA6C0CCCD7BF369@SA1PR21MB1335.namprd21.prod.outlook.com>
Date:   Tue, 1 Nov 2022 01:58:24 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     Stefano Garzarella <sgarzare@...hat.com>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "arseny.krasnov@...persky.com" <arseny.krasnov@...persky.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        "stephen@...workplumber.org" <stephen@...workplumber.org>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>
Subject: RE: [PATCH 2/2] vsock: fix possible infinite sleep in
 vsock_connectible_wait_data()

> From: Stefano Garzarella <sgarzare@...hat.com>
> Sent: Monday, October 31, 2022 1:43 AM
>  ...
> s/qeuue/queue
Will fix this.
 
> >@@ -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?

It's not needed. Will remove this and send v2.

> 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

I'd like to stay with my version, as it only needs one line of
prepare_to_wait(), and IMO it's more readable if we only exit from
inside the while loop.


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ