linux-kernel - RE: [PATCH v3] vfio: fix deadlock between group lock and kvm lock

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]

Message-ID: <BN9PR11MB5276017F9CEBB4BAE58C40E88CD79@BN9PR11MB5276.namprd11.prod.outlook.com>
Date:   Fri, 3 Feb 2023 02:00:21 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Alex Williamson <alex.williamson@...hat.com>
CC:     Matthew Rosato <mjrosato@...ux.ibm.com>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "Liu, Yi L" <yi.l.liu@...el.com>,
        "jgg@...dia.com" <jgg@...dia.com>,
        "cohuck@...hat.com" <cohuck@...hat.com>,
        "farman@...ux.ibm.com" <farman@...ux.ibm.com>,
        "pmorel@...ux.ibm.com" <pmorel@...ux.ibm.com>,
        "borntraeger@...ux.ibm.com" <borntraeger@...ux.ibm.com>,
        "frankja@...ux.ibm.com" <frankja@...ux.ibm.com>,
        "imbrenda@...ux.ibm.com" <imbrenda@...ux.ibm.com>,
        "david@...hat.com" <david@...hat.com>,
        "akrowiak@...ux.ibm.com" <akrowiak@...ux.ibm.com>,
        "jjherne@...ux.ibm.com" <jjherne@...ux.ibm.com>,
        "pasic@...ux.ibm.com" <pasic@...ux.ibm.com>,
        "zhenyuw@...ux.intel.com" <zhenyuw@...ux.intel.com>,
        "Wang, Zhi A" <zhi.a.wang@...el.com>,
        "Christopherson,, Sean" <seanjc@...gle.com>,
        "linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "intel-gvt-dev@...ts.freedesktop.org" 
        <intel-gvt-dev@...ts.freedesktop.org>,
        "intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v3] vfio: fix deadlock between group lock and kvm lock

> From: Alex Williamson <alex.williamson@...hat.com>
> Sent: Friday, February 3, 2023 7:13 AM
> 
> On Thu, 2 Feb 2023 23:04:10 +0000
> "Tian, Kevin" <kevin.tian@...el.com> wrote:
> 
> > > From: Alex Williamson <alex.williamson@...hat.com>
> > > Sent: Friday, February 3, 2023 3:42 AM
> > >
> > >
> > > LGTM.  I'm not sure moving the functions to vfio_main really buys us
> > > anything since we're making so much use of group fields.  The cdev
> > > approach will necessarily be different, so the bulk of the get code will
> > > likely need to move back to group.c anyway.
> > >
> >
> > well my last comment was based on Matthew's v2 where the get code
> > gets a kvm passed in instead of implicitly retrieving group ref_lock
> > internally. In that case the get/put helpers only contain device logic
> > thus fit in vfio_main.c.
> >
> > with v3 then they have to be in group.c since we don't want to use
> > group fields in vfio_main.c.
> >
> > but I still think v2 of the helpers is slightly better. The only difference
> > between cdev and group when handling this race is using different
> > ref_lock. the symbol get/put part is exactly same. So even if we
> > merge v3 like this, very likely Yi has to change it back to v2 style
> > to share the get/put helpers while just leaving the ref_lock part
> > handled differently between the two path.
> 
> I'm not really a fan of the asymmetry of the v2 version where the get
> helper needs to be called under the new kvm_ref_lock, but the put
> helper does not.  Having the get helper handle that makes the caller
> much cleaner.  Thanks,
> 

What about passing the lock pointer into the helper? it's still slightly
asymmetry as the put helper doesn't carry the lock pointer but it
could also be interpreted as if the pointer has been saved in the get
then if it needs to be referenced by the put there is no need to pass
it in again.