linux-kernel - Re: [PATCH v2 1/2] vfio/type1: Simplify bus

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, 24 Jun 2022 11:18:36 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Alex Williamson <alex.williamson@...hat.com>
Cc:     Robin Murphy <robin.murphy@....com>, cohuck@...hat.com,
        iommu@...ts.linux.dev, iommu@...ts.linux-foundation.org,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] vfio/type1: Simplify bus_type determination

On Fri, Jun 24, 2022 at 08:11:59AM -0600, Alex Williamson wrote:
> On Thu, 23 Jun 2022 22:50:30 -0300
> Jason Gunthorpe <jgg@...dia.com> wrote:
> 
> > On Thu, Jun 23, 2022 at 05:00:44PM -0600, Alex Williamson wrote:
> > 
> > > > >> +struct vfio_device *vfio_device_get_from_iommu(struct iommu_group *iommu_group)
> > > > >> +{
> > > > >> +	struct vfio_group *group = vfio_group_get_from_iommu(iommu_group);
> > > > >> +	struct vfio_device *device;    
> > > > > 
> > > > > Check group for NULL.    
> > > > 
> > > > OK - FWIW in context this should only ever make sense to call with an 
> > > > iommu_group which has already been derived from a vfio_group, and I did 
> > > > initially consider a check with a WARN_ON(), but then decided that the 
> > > > unguarded dereference would be a sufficiently strong message. No problem 
> > > > with bringing that back to make it more defensive if that's what you prefer.  
> > > 
> > > A while down the road, that's a bit too much implicit knowledge of the
> > > intent and single purpose of this function just to simply avoid a test.  
> > 
> > I think we should just pass the 'struct vfio_group *' into the
> > attach_group op and have this API take that type in and forget the
> > vfio_group_get_from_iommu().
> 
> That's essentially what I'm suggesting, the vfio_group is passed as an
> opaque pointer which type1 can use for a
> vfio_group_for_each_vfio_device() type call.  Thanks,

I don't want to add a whole vfio_group_for_each_vfio_device()
machinery that isn't actually needed by anything.. This is all
internal, we don't need to design more than exactly what is needed.

At this point if we change the signature of the attach then we may as
well just pass in the representative vfio_device, that is probably
less LOC overall.

Jason