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@xxxxxxxxxxxxx> 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