Re: [PATCH v3 2/6] userfaultfd: add /dev/userfaultfd for fine grained access control

[Axel Rasmussen <axelrasmussen@google.com>]

On Mon, Jun 13, 2022 at 3:29 PM Peter Xu <peterx@redhat.com> wrote:
>
> On Mon, Jun 13, 2022 at 02:55:40PM -0700, Andrew Morton wrote:
> > On Wed,  1 Jun 2022 14:09:47 -0700 Axel Rasmussen <axelrasmussen@google.com> wrote:
> >
> > > To achieve this, add a /dev/userfaultfd misc device. This device
> > > provides an alternative to the userfaultfd(2) syscall for the creation
> > > of new userfaultfds. The idea is, any userfaultfds created this way will
> > > be able to handle kernel faults, without the caller having any special
> > > capabilities. Access to this mechanism is instead restricted using e.g.
> > > standard filesystem permissions.
> >
> > The use of a /dev node isn't pretty.  Why can't this be done by
> > tweaking sys_userfaultfd() or by adding a sys_userfaultfd2()?

I think for any approach involving syscalls, we need to be able to
control access to who can call a syscall. Maybe there's another way
I'm not aware of, but I think today the only mechanism to do this is
capabilities. I proposed adding a CAP_USERFAULTFD for this purpose,
but that approach was rejected [1]. So, I'm not sure of another way
besides using a device node.

One thing that could potentially make this cleaner is, as one LWN
commenter pointed out, we could have open() on /dev/userfaultfd just
return a new userfaultfd directly, instead of this multi-step process
of open /dev/userfaultfd, NEW ioctl, then you get a userfaultfd. When
I wrote this originally it wasn't clear to me how to get that to
happen - open() doesn't directly return the result of our custom open
function pointer, as far as I can tell - but it could be investigated.

[1]: https://lore.kernel.org/lkml/686276b9-4530-2045-6bd8-170e5943abe4@schaufler-ca.com/T/

> >
> > Peter, will you be completing review of this patchset?
>
> Sorry to not have reviewed it proactively..
>
> I think it's because I never had a good picture/understanding of what
> should be the best security model for uffd, meanwhile I am (it seems) just
> seeing more and more ways to "provide a safer uffd" by different people
> using different ways.. and I never had time (and probably capability too..)
> to figure out the correct approach if not to accept all options provided.

Agreed, what we have right now is a bit of a mess of different
approaches. I think the reason for this is, there is no "perfect" way
to control access to features like this, so what we now have is
several different approaches with different tradeoffs.

From my perspective, the existing controls were simpler to implement,
but are not ideal because they require us to grant access to UFFD
*plus more stuff too*.

The approach I've proposed is the most granular, so it doesn't require
adding any extra permissions. But, I agree the interface is sort of
overcomplicated. :/ But, from my perspective, security in shared Cloud
computing environments where UFFD is used for live migration is
critical, so I prefer this tradeoff - I'll put up with a slightly
messier interface, if the gain is a very minimal set of privileges.

>
> I think I'll just assume the whole thing is acked already from you
> generally, then I'll read at least the implementation before the end of
> tomorrow.
>
> Thanks,
>
> --
> Peter Xu
>