Re: [PATCH V4 0/5] fixes for virtual address update

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Dec 14, 2022 at 02:42:29PM -0700, Alex Williamson wrote:
> On Wed, 14 Dec 2022 13:24:52 -0800
> Steve Sistare <steven.sistare@oracle.com> wrote:
> 
> > Fix bugs in the interfaces that allow the underlying memory object of an
> > iova range to be mapped in a new address space.  They allow userland to
> > indefinitely block vfio mediated device kernel threads, and do not
> > propagate the locked_vm count to a new mm.
> > 
> > The fixes impose restrictions that eliminate waiting conditions, so
> > revert the dead code:
> >   commit 898b9eaeb3fe ("vfio/type1: block on invalid vaddr")
> >   commit 487ace134053 ("vfio/type1: implement notify callback")
> >   commit ec5e32940cc9 ("vfio: iommu driver notify callback")
> > 
> > Changes in V2 (thanks Alex):
> >   * do not allow group attach while vaddrs are invalid
> >   * add patches to delete dead code
> >   * add WARN_ON for never-should-happen conditions
> >   * check for changed mm in unmap.
> >   * check for vfio_lock_acct failure in remap
> > 
> > Changes in V3 (ditto!):
> >   * return errno at WARN_ON sites, and make it unique
> >   * correctly check for dma task mm change
> >   * change dma owner to current when vaddr is updated
> >   * add Fixes to commit messages
> >   * refactored new code in vfio_dma_do_map
> > 
> > Changes in V4:
> >   * misc cosmetic changes
> > 
> > Steve Sistare (5):
> >   vfio/type1: exclude mdevs from VFIO_UPDATE_VADDR
> >   vfio/type1: prevent locked_vm underflow
> >   vfio/type1: revert "block on invalid vaddr"
> >   vfio/type1: revert "implement notify callback"
> >   vfio: revert "iommu driver notify callback"
> > 
> >  drivers/vfio/container.c        |   5 -
> >  drivers/vfio/vfio.h             |   7 --
> >  drivers/vfio/vfio_iommu_type1.c | 199 +++++++++++++++++++---------------------
> >  include/uapi/linux/vfio.h       |  15 +--
> >  4 files changed, 103 insertions(+), 123 deletions(-)
> > 
> 
> Looks ok to me.  Jason, Kevin, I'd appreciate your reviews regarding
> whether this sufficiently addresses the outstanding issues to keep this
> interface around until we have a re-implementation in iommufd.  Thanks,

Well, patch 3 undeniably solves the deadlock problem.

I still don't like that we are keeping this, an interface that doesn't
support mdevs has no future and can't really be used in any general
purpose way.

Can we at least protect it with a kconfig && CONFIG_EXPERIMENTAL so it
is disabled in most builds?

Jason