netdev - RE: [PATCH net v2] vsock: Fix a lockdep warning in __vsock

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 27 Sep 2019 05:37:20 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     Stefano Garzarella <sgarzare@...hat.com>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "sashal@...nel.org" <sashal@...nel.org>,
        "stefanha@...hat.com" <stefanha@...hat.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "arnd@...db.de" <arnd@...db.de>,
        "deepa.kernel@...il.com" <deepa.kernel@...il.com>,
        "ytht.net@...il.com" <ytht.net@...il.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        Michael Kelley <mikelley@...rosoft.com>,
        "jhansen@...are.com" <jhansen@...are.com>
Subject: RE: [PATCH net v2] vsock: Fix a lockdep warning in __vsock_release()

> From: linux-hyperv-owner@...r.kernel.org
> <linux-hyperv-owner@...r.kernel.org> On Behalf Of Stefano Garzarella
> Sent: Thursday, September 26, 2019 12:48 AM
> 
> Hi Dexuan,
> 
> On Thu, Sep 26, 2019 at 01:11:27AM +0000, Dexuan Cui wrote:
> > ...
> > NOTE: I only tested the code on Hyper-V. I can not test the code for
> > virtio socket, as I don't have a KVM host. :-( Sorry.
> >
> > @Stefan, @Stefano: please review & test the patch for virtio socket,
> > and let me know if the patch breaks anything. Thanks!
> 
> Comment below, I'll test it ASAP!

Stefano, Thank you!

BTW, this is how I tested the patch:
1. write a socket server program in the guest. The program calls listen()
and then calls sleep(10000 seconds). Note: accept() is not called.

2. create some connections to the server program in the guest.

3. kill the server program by Ctrl+C, and "dmesg" will show the scary
call-trace, if the kernel is built with 
	CONFIG_LOCKDEP=y
	CONFIG_LOCKDEP_SUPPORT=y

4. Apply the patch, do the same test and we should no longer see the call-trace.

> > -		lock_sock(sk);
> > +		/* When "level" is 2, use the nested version to avoid the
> > +		 * warning "possible recursive locking detected".
> > +		 */
> > +		if (level == 1)
> > +			lock_sock(sk);
> 
> Since lock_sock() calls lock_sock_nested(sk, 0), could we use directly
> lock_sock_nested(sk, level) with level = 0 in vsock_release() and
> level = SINGLE_DEPTH_NESTING here in the while loop?
> 
> Thanks,
> Stefano

IMHO it's better to make the lock usage more explicit, as the patch does.

lock_sock_nested(sk, level) or lock_sock_nested(sk, 0) seems a little
odd to me. But I'm open to your suggestion: if any of the network
maintainers, e.g. davem, also agrees with you, I'll change the code 
as you suggested. :-)

Thanks,
-- Dexuan