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

Jason Gunthorpe
2022-11-09T01:05:28Z

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..

> > I'd be much more comfortable with this as a system wide iommufd flag
> > if we also tied it to do some demonstration of privilege - eg a
> > requirement to open iommufd with CAP_SYS_RAWIO for instance.
>
> Which is not compatible to existing use cases, which is also why we
> can't invent some way to allow some applications to run without CPU
> mitigations, while requiring it for others as a baseline.

Isn't it? Didn't we learn that libvirt runs as root and will open and
pass the iommufd as root?

> > That is the usual protocol for these kinds of insecurities..
>
> Hmm, is it?

I think so. At least you should have something to shut down an
insecure feature in kernel lockdown modes. CAP_SYS_RAWIO is a simple
way to do it.

> > I think right now we can leave this as-is and we can wait for some
> > more information to decide how best to proceed.
>
> It's certainly not acceptable in the latest proposal, iommufd consumes
> an option set by another module and when that module goes away, so does
> any claim of compatibility. The code becomes dead and the feature not
> present. The option doesn't belong on the vfio module. Do we need a
> vfio-iommufd module to host it? Thanks,

I don't know, as I said in the other email, these little things need
work and discussion to resolve. We need to recheck the security stuff
against the 2022 kernel where things have changed. We don't need to do
it all right now.

People who want allow_unsafe_interrupts to work will simply not set
VFIO_CONTAINER=n at this time. Same with P2P, vfio-no-iommu and any
other gaps we haven't discovered.

vfio-iommufd seems like overkill, I think your first suggestion to put
in vfio.ko was more practical.

My only doubt is if we should make it system wide for everything - and
I'm just a bit uncomfortable with that from a security POV. But maybe
I don't quite know exactly what the risks are.

Jason