Re: [PATCH RFC v2 00/13] IOMMUFD Generic interface
On Wed, Sep 21, 2022 at 12:06:49PM -0600, Alex Williamson wrote:
> I still think the compat gaps are small. I've realized that
> VFIO_DMA_UNMAP_FLAG_VADDR has no implementation in qemu, and since it
> can deadlock the kernel I propose we purge it completely.
Steve won't be happy to hear that, QEMU support exists but isn't yet
merged.
If Steve wants to keep it then someone needs to fix the deadlock in
the vfio implementation before any userspace starts to appear.
I can fix the deadlock in iommufd in a terrible expensive way, but
would rather we design a better interface if nobody is using it yet. I
advocate for passing the memfd to the kernel and use that as the page
provider, not a mm_struct.
The issue is where we account these pinned pages, where accounting
is
necessary such that a user cannot lock an arbitrary number of pages
into RAM to generate a DoS attack.
It is worth pointing out that preventing a DOS attack doesn't actually
work because a *task* limit is trivially bypassed by just spawning
more tasks. So, as a security feature, this is already very
questionable.
What we've done here is make the security feature work to actually
prevent DOS attacks, which then gives you this problem:
This obviously has implications. AFAICT, any management tool that
doesn't instantiate assigned device VMs under separate users are
essentially untenable.
Because now that the security feature works properly it detects the
DOS created by spawning multiple tasks :(
Somehow I was under the impression there was not user sharing in the
common cases, but I guess I don't know that for sure.
> So, I still like 2 because it yields the smallest next step
before we
> can bring all the parallel work onto the list, and it makes testing
> and converting non-qemu stuff easier even going forward.
If a vfio compatible interface isn't transparently compatible, then I
have a hard time understanding its value. Please correct my above
description and implications, but I suspect these are not just
theoretical ABI compat issues. Thanks,
Because it is just fine for everything that doesn't use the ulimit
feature, which is still a lot of use cases!
Remember, at this point we are not replacing /dev/vfio/vfio, this is
just providing the general compat in a form that has to be opted
into. I think if you open the /dev/iommu device node then you should
get secured accounting.
If /dev/vfio/vfio is provided by iommufd it may well have to trigger a
different ulimit tracking - if that is the only sticking point it
seems minor and should be addressed in some later series that adds
/dev/vfio/vfio support to iommufd..
Jason