Re: [RFC PATCH v3 1/2] mm: restrictedmem: Allow userspace to specify mount for memfd_restricted

[Ackerley Tng]

Thanks for reviewing these patches!

"Kirill A. Shutemov" <kirill@shutemov.name> writes:

> On Fri, Mar 31, 2023 at 11:50:39PM +0000, Ackerley Tng wrote:

> > ...

> > +static int restrictedmem_create_on_user_mount(int mount_fd)
> > +{
> > +       int ret;
> > +       struct fd f;
> > +       struct vfsmount *mnt;
> > +
> > +       f = fdget_raw(mount_fd);
> > +       if (!f.file)
> > +               return -EBADF;
> > +
> > +       ret = -EINVAL;
> > +       if (!is_mount_root(f.file))
> > +               goto out;
> > +
> > +       mnt = f.file->f_path.mnt;
> > +       if (!is_shmem_mount(mnt))
> > +               goto out;
> > +
> > +       ret = file_permission(f.file, MAY_WRITE | MAY_EXEC);

> Why MAY_EXEC?


Christian pointed out that this check does not make sense, I'll be
removing the entire check in the next revision.

> > +       if (ret)
> > +               goto out;
> > +
> > +       ret = mnt_want_write(mnt);
> > +       if (unlikely(ret))
> > +               goto out;
> > +
> > +       ret = restrictedmem_create(mnt);
> > +
> > +       mnt_drop_write(mnt);
> > +out:
> > +       fdput(f);
> > +
> > +       return ret;
> > +}

> We need review from fs folks. Look mostly sensible, but I have no
> experience in fs.

> > +
> > +SYSCALL_DEFINE2(memfd_restricted, unsigned int, flags, int, mount_fd)
> > +{
> > +       if (flags & ~RMFD_USERMNT)
> > +               return -EINVAL;
> > +
> > +       if (flags == RMFD_USERMNT) {
> > +               if (mount_fd < 0)
> > +                       return -EINVAL;
> > +
> > +               return restrictedmem_create_on_user_mount(mount_fd);
> > +       } else {
> > +               return restrictedmem_create(NULL);
> > +       }

> Maybe restructure with single restrictedmem_create() call?

>         struct vfsmount *mnt = NULL;

>         if (flags == RMFD_USERMNT) {
>                 ...
>                 mnt = ...();
>         }

>         return restrictedmem_create(mnt);

Will do so in the next revision.

> > +}
> > +
> >   int restrictedmem_bind(struct file *file, pgoff_t start, pgoff_t end,
> >                        struct restrictedmem_notifier *notifier, bool exclusive)
> >   {
> > --
> > 2.40.0.348.gf938b09366-goog