linux-kernel - Re: [PATCH 2/2] s390/vfio-ap: replace open coded locks for VFIO_GROUP_NOTIFY_SET

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:   Tue, 27 Jul 2021 07:54:40 +0100
From:   Christoph Hellwig <hch@...radead.org>
To:     Jason Gunthorpe <jgg@...dia.com>
Cc:     Halil Pasic <pasic@...ux.ibm.com>,
        Tony Krowiak <akrowiak@...ux.ibm.com>,
        linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
        borntraeger@...ibm.com, cohuck@...hat.com,
        pasic@...ux.vnet.ibm.com, jjherne@...ux.ibm.com,
        alex.williamson@...hat.com, kwankhede@...dia.com, david@...hat.com,
        kvm@...r.kernel.org
Subject: Re: [PATCH 2/2] s390/vfio-ap: replace open coded locks for
 VFIO_GROUP_NOTIFY_SET_KVM notification

On Mon, Jul 26, 2021 at 07:03:17PM -0300, Jason Gunthorpe wrote:
> On Mon, Jul 26, 2021 at 10:36:28PM +0200, Halil Pasic wrote:
> 
> > You may end up with open and close running interleaved. What I'
> > trying to say is, to my best knowledge, normally there is no
> > you have to close it before you open it again rule for files.
> 
> This is an existing bug in this driver, I've fixed in the reflck series.
> 
> open_device/close_device will not run concurrently, or out of order,
> afer it is fixed.

Btw, while I've got all your attention, I've been struggling a bit with
how that SET_KVM notifier is supposed to work.

The i915 gvt code simplify assumes the notification registration hits
the case of KVM already being active, and gets away with that as at
least qemu ensures that the KVM_DEV_VFIO_GROUP_ADD has been called before
the device FDs are opened.  Is that something we could generalize and
never allow to actually notify for SET_KVM with non-null data beeing
called at runtime and avoid the locking entirely?

Similarly the removal case is a little weird: with Jason's work to only
call a release function on the last reference drop which solves a lot
of concurrency issues nicely  this still creates a nasty corner case
with a sideband release under weird locking rules.   What prevents the
vfio core from simply holding a refefeence to the struct kvm as long as
the device is open to close that hole?