Re: [Intel-gfx] [PATCH 04/10] vfio: Move storage of allow_unsafe_interrupts to vfio_main.c

Jason Gunthorpe
2022-11-09T13:11:50Z

On Wed, Nov 09, 2022 at 03:21:29AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Wednesday, November 9, 2022 9:05 AM
> >
> > On Tue, Nov 08, 2022 at 03:55:20PM -0700, Alex Williamson wrote:
> >
> > > > > So why exactly isn't this an issue for VDPA? Are we just burying our
> > > > > head in the sand that such platforms exists and can still be useful
> > > > > given the appropriate risk vs reward trade-off?
> > > >
> > > > Simply that nobody has asked for it, and might never ask for it. This
> > > > is all support for old platforms, and there just doesn't seem to be a
> > > > "real" use case for very new (and actually rare) NIC hardware stuck
> > > > into ancient platforms with this security problem.
> > >
> > > vIOMMU support for interrupt remapping is relatively new, the nesting
> > > case is important as well.
> >
> > This is where we got hit. In the end we fixed the qemu..
>
> but the point is that old qemu could have a much longer lifespan than
> old platforms then when running newer kernel which supports vdpa
> on old qemu the same tradeoff consideration is then not vfio
> specific...

I think we are reaching into incredible hypotheticals here. We don't
know of any real uses cases where a very new VDPA capable device would
be assinged into a VM using an old qemu and the entire system is
expected to work. What we are seeing is people using this stuff are
making highly engineered systems and will meet the constraints.

Today VDPA doesn't support allow_unsafe_interrupts, and I don't see a
compelling reason to change that.

The threshold for introducing a kernel security hole should be much
higher than "someone could possibly do this".

> If all agree that VFIO_CONTAINER=n is a process to evolve, does it make
> more sense to remove this patch from this series i.e. let it buried in
> VFIO_CONTAINER=y for now? Then resolve it in a follow up patch if
> no consensus can be made quickly at this point.

This is worse, it would make iommufd completely unusable in situations
where we need allow_unsafe_interrupts. If we belive that is important
we should keep this patch so existing systems on kernels with
VFIO_CONTAINER=y continue to work after libvirt/qemu are upgraded to
iommufd.

Jason