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

Ackerley Tng
2023-04-05T22:32:58Z

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