Re: [PATCH] vfio/type1: Cleanup remaining vaddr removal/update fragments

[Jason Gunthorpe <jgg@ziepe.ca>]

On Mon, Dec 12, 2022 at 04:29:48PM -0700, Alex Williamson wrote:
> On Mon, 12 Dec 2022 19:08:57 -0400
> Jason Gunthorpe <jgg@ziepe.ca> wrote:
> 
> > On Mon, Dec 12, 2022 at 02:26:51PM -0700, Alex Williamson wrote:
> > > On Mon, 12 Dec 2022 15:59:11 -0500
> > > Steven Sistare <steven.sistare@oracle.com> wrote:
> > >   
> > > > On 12/12/2022 10:58 AM, Alex Williamson wrote:  
> > > > > On Mon, 12 Dec 2022 09:17:54 -0400
> > > > > Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > > > >     
> > > > >> On Sat, Dec 10, 2022 at 09:14:06AM -0500, Steven Sistare wrote:
> > > > >>    
> > > > >>> Thank you for your thoughtful response.  Rather than debate the degree of
> > > > >>> of vulnerability, I propose an alternate solution.  The technical crux of
> > > > >>> the matter is support for mediated devices.        
> > > > >>
> > > > >> I'm not sure I'm convinced about that. It is easy to make problematic
> > > > >> situations with mdevs, but that doesn't mean other cases don't exist
> > > > >> too eg what happens if userspace suspends and then immediately does
> > > > >> something to trigger a domain attachment? Doesn't it still deadlock
> > > > >> the kernel?    
> > > > > 
> > > > > The opportunity for that to deadlock isn't obvious to me, a replay
> > > > > would be stalled waiting for invalid vaddrs, but this is essentially
> > > > > the user deadlocking themselves.  There's also code there to handle the
> > > > > process getting killed while waiting, making it interruptible.  Thanks,    
> > > > 
> > > > I will submit new patches tomorrow to exclude mdevs.  Almost done.  
> > > 
> > > I've dropped the removal commits from my next branch in the interim.  
> > 
> > Woah, please don't do that - I already built and sent pull requests
> > assuming this, there are conflicts.
> 
> I've done merges both ways with your iommufd pull request and don't see
> any conflicts relative to these changes.  Kconfig, Makefile, and
> vfio_main.c related to virq integration and group extraction are the
> only conflicts. 

I got an extra hunk in the header file

> Besides, it's already pushed and I don't have any references to the
> old head, so someone would need to provide it if we wanted to keep
> the old hashes.

https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/tag/?h=for-linus-iommufd-merged
https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/commit/?h=for-linus-iommufd-merged&id=e9a1f0f32d86c05f01878a0448384a46a453abc7

> > Why would we not revert everything from 6.2 - that is what we agreed
> > to do?
> 
> The decision to revert was based on the current interface being buggy,
> abandoned, and re-implemented.  It doesn't seem that there's much future
> for the current interface, but Steve has stepped up to restrict the
> current implementation to non-mdev devices, which resolves your concern
> regarding unlimited user blocking of kernel threads afaict, and we'll
> see what he does with locked memory.  

Except nobody has seen this yet, and it can't go into 6.2 at this
point (see Linus's rather harsh remarks on late work for v6.2)

So we are punting on this for another kernel cycle. I don't like any
of this.

Regardless of what happens I need this all settled and the vfio tree
unchanging by Thursday to finalize the above tag..

Jason