On Wed, Oct 23, 2019 at 5:44 AM Mike Rapoport <rppt@xxxxxxxxxxxxx> wrote: > > On Wed, Oct 23, 2019 at 10:29:20AM +0300, Cyrill Gorcunov wrote: > > On Tue, Oct 22, 2019 at 09:11:04PM -0700, Andy Lutomirski wrote: > > > Trying again. It looks like I used the wrong address for Pavel. > > > > Thanks for CC Andy! I must confess I didn't dive into userfaultfd engine > > personally but let me CC more people involved from criu side. (overquoting > > left untouched for their sake). > > Thanks for CC Cyrill! > > > > > On Sat, Oct 12, 2019 at 6:14 PM Andy Lutomirski <luto@xxxxxxxxxx> wrote: > > > > > > > > [adding more people because this is going to be an ABI break, sigh] > > > > > > > > On Sat, Oct 12, 2019 at 5:52 PM Daniel Colascione <dancol@xxxxxxxxxx> wrote: > > > > > > > > > > On Sat, Oct 12, 2019 at 4:10 PM Andy Lutomirski <luto@xxxxxxxxxx> wrote: > > > > > > > > > > > > On Sat, Oct 12, 2019 at 12:16 PM Daniel Colascione <dancol@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > The new secure flag makes userfaultfd use a new "secure" anonymous > > > > > > > file object instead of the default one, letting security modules > > > > > > > supervise userfaultfd use. > > > > > > > > > > > > > > Requiring that users pass a new flag lets us avoid changing the > > > > > > > semantics for existing callers. > > > > > > > > > > > > Is there any good reason not to make this be the default? > > > > > > > > > > > > > > > > > > The only downside I can see is that it would increase the memory usage > > > > > > of userfaultfd(), but that doesn't seem like such a big deal. A > > > > > > lighter-weight alternative would be to have a single inode shared by > > > > > > all userfaultfd instances, which would require a somewhat different > > > > > > internal anon_inode API. > > > > > > > > > > I'd also prefer to just make SELinux use mandatory, but there's a > > > > > nasty interaction with UFFD_EVENT_FORK. Adding a new UFFD_SECURE mode > > > > > which blocks UFFD_EVENT_FORK sidesteps this problem. Maybe you know a > > > > > better way to deal with it. > > > > > > > > > > Right now, when a process with a UFFD-managed VMA using > > > > > UFFD_EVENT_FORK forks, we make a new userfaultfd_ctx out of thin air > > > > > and enqueue it on the message queue for the parent process. When we > > > > > dequeue that context, we get to resolve_userfault_fork, which makes up > > > > > a new UFFD file object out of thin air in the context of the reading > > > > > process. Following normal SELinux rules, the SID attached to that new > > > > > file object would be the task SID of the process *reading* the fork > > > > > event, not the SID of the new fork child. That seems wrong, because > > > > > the label we give to the UFFD should correspond to the label of the > > > > > process that UFFD controls. > > I must admit I have no idea about how SELinux works, but what's wrong with > making the new UFFD object to inherit the properties of the "original" one? > > The new file object is created in the context of the same task that owns > the initial userfault file descriptor and it is used by the same task. So > if you have a process that registers some of its VMAs with userfaultfd > and enables UFFD_EVENT_FORK, the same process controls UFFD of itself and > its children. I'm not actually convinced this is a problem. What *is* a problem is touching the file descriptor table at all from read(2). That's a big no-no. --Andy
