RE: [PATCH v10 1/9] mm: Introduce memfd_restricted system call to create restricted user memory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Monday, January 30, 2023 1:26 PM, Ackerley Tng wrote:
> 
> > +static int restrictedmem_getattr(struct user_namespace *mnt_userns,
> > +				 const struct path *path, struct kstat *stat,
> > +				 u32 request_mask, unsigned int query_flags)
> {
> > +	struct inode *inode = d_inode(path->dentry);
> > +	struct restrictedmem_data *data = inode->i_mapping-
> >private_data;
> > +	struct file *memfd = data->memfd;
> > +
> > +	return memfd->f_inode->i_op->getattr(mnt_userns, path, stat,
> > +					     request_mask, query_flags);
> 
> Instead of calling shmem's getattr() with path, we should be using the the
> memfd's path.
> 
> Otherwise, shmem's getattr() will use restrictedmem's inode instead of
> shmem's inode. The private fields will be of the wrong type, and the host will
> crash when shmem_is_huge() does SHMEM_SB(inode->i_sb)->huge), since
> inode->i_sb->s_fs_info is NULL for the restrictedmem's superblock.
> 
> Here's the patch:
> 
> diff --git a/mm/restrictedmem.c b/mm/restrictedmem.c index
> 37191cd9eed1..06b72d593bd8 100644
> --- a/mm/restrictedmem.c
> +++ b/mm/restrictedmem.c
> @@ -84,7 +84,7 @@ static int restrictedmem_getattr(struct user_namespace
> *mnt_userns,
>   	struct restrictedmem *rm = inode->i_mapping->private_data;
>   	struct file *memfd = rm->memfd;
> 
> -	return memfd->f_inode->i_op->getattr(mnt_userns, path, stat,
> +	return memfd->f_inode->i_op->getattr(mnt_userns, &memfd-
> >f_path, stat,
>   					     request_mask, query_flags);
>   }
> 

Nice catch. I also encountered this issue during my work.
The fix can further be enforced by shmem:

index c301487be5fb..d850c0190359 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -472,8 +472,9 @@ bool shmem_is_huge(struct vm_area_struct *vma, struct inode *inode,
                   pgoff_t index, bool shmem_huge_force)
 {
        loff_t i_size;
+       struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb);

-       if (!S_ISREG(inode->i_mode))
+       if (!sbinfo || !S_ISREG(inode->i_mode))
                return false;
        if (vma && ((vma->vm_flags & VM_NOHUGEPAGE) ||
            test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags)))
@@ -485,7 +486,7 @@ bool shmem_is_huge(struct vm_area_struct *vma, struct inode *inode,
        if (shmem_huge == SHMEM_HUGE_DENY)
                return false;

-       switch (SHMEM_SB(inode->i_sb)->huge) {
+       switch (sbinfo->huge) {
        case SHMEM_HUGE_ALWAYS:
                return true;
        case SHMEM_HUGE_WITHIN_SIZE:




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux