On 28.04.23 17:27, Jason Gunthorpe wrote:
On Fri, Apr 28, 2023 at 05:08:27PM +0200, David Hildenbrand wrote:
I think this is broken today and we should block it. We know from
experiments with RDMA that doing exactly this triggers kernel oop's.
I never saw similar reports in the wild (especially targeted at RHEL), so is
this still a current issue that has not been mitigated? Or is it just so
hard to actually trigger?
People send RDMA related bug reports to us, and we tell them not to do
this stuff :)
I'm skeptical that anyone can actually do this combination of things
successfully without getting kernel crashes or file data corruption -
ie there is no real user to break.
I am pretty sure that there are such VM users, because on the libvirt level
it's completely unclear which features trigger what behavior :/
IDK, why on earth would anyone want to do this? Using VFIO forces all
the memory to become resident so what was the point of making it file
backed in the first place?
As I said, copy-and paste, incremental changes to domain XMLs. I've seen
some crazy domain XMLs in bug reports.
I'm skeptical there are real users even if it now requires special
steps to be crashy/corrupty.
In any case, I think we should document the possible implications of
this patch. I gave one use case that could be broken.
Sure, we could warn, or convert individual users using a flag (io_uring).
But maybe we should invest more energy on a fix?
It has been years now, I think we need to admit a fix is still years
away. Blocking the security problem may even motivate more people to
work on a fix.
Maybe we should make this a topic this year at LSF/MM (again?). At least we
learned a lot about GUP, what might work, what might not work, and got a
depper understanding (+ motivation to fix? :) ) the issue at hand.
We keep having the topic.. This is the old argument that the FS people
say the MM isn't following its inode and dirty lifetime rules and the
MM people say the FS isn't following its refcounting rules <shrug>
:/ so we have to discuss it ... again I guess.
Security is the primary case where we have historically closed uAPI
items.
As this patch
1) Does not tackle GUP-fast
2) Does not take care of !FOLL_LONGTERM
I am not convinced by the security argument in regard to this patch.
It is incremental and a temperature check to see what kind of real
users exist. We have no idea right now, just speculation.
Right, but again, if we start talking about security it's a different
thing IMHO.
Everything else sounds like band-aids to me, is insufficient, and might
cause more harm than actually help IMHO. Especially the gup-fast case is
extremely easy to work-around in malicious user space.
It is true this patch should probably block gup_fast when using
FOLL_LONGTERM as well, just like we used to do for the DAX check.
Then we'd at least fix the security issue for all FOLL_LONGTERM completely.
--
Thanks,
David / dhildenb
