Re: [PATCH 08/15] xfs: don't modify file and inode flags for shmem files

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Jan 03, 2024 at 08:41:19AM +0000, Christoph Hellwig wrote:
> shmem_file_setup is explicitly intended for a file that can be
> fully read and written by kernel users without restrictions.  Don't
> poke into internals to change random flags in the file or inode.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/scrub/xfile.c | 15 ---------------
>  1 file changed, 15 deletions(-)
> 
> diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c
> index ec1be08937977a..e872f4f0263f59 100644
> --- a/fs/xfs/scrub/xfile.c
> +++ b/fs/xfs/scrub/xfile.c
> @@ -74,22 +74,7 @@ xfile_create(
>  		goto out_xfile;
>  	}
>  
> -	/*
> -	 * We want a large sparse file that we can pread, pwrite, and seek.
> -	 * xfile users are responsible for keeping the xfile hidden away from
> -	 * all other callers, so we skip timestamp updates and security checks.
> -	 * Make the inode only accessible by root, just in case the xfile ever
> -	 * escapes.
> -	 */
> -	xf->file->f_mode |= FMODE_PREAD | FMODE_PWRITE | FMODE_NOCMTIME |
> -			    FMODE_LSEEK;
> -	xf->file->f_flags |= O_RDWR | O_LARGEFILE | O_NOATIME;
>  	inode = file_inode(xf->file);
> -	inode->i_flags |= S_PRIVATE | S_NOCMTIME | S_NOATIME;

I actually want S_PRIVATE here to avoid interference from all the
security hooks and whatnot when scrub is using an xfile to stash a
large amount of data.  Shouldn't this patch change xfile_create to call
shmem_kernel_file_setup instead?

> -	inode->i_mode &= ~0177;
> -	inode->i_uid = GLOBAL_ROOT_UID;
> -	inode->i_gid = GLOBAL_ROOT_GID;

Also, I don't know if it matters that the default uid/gid are now going
to be whatever the defaults would be for a new file instead of root
only.  That seems like it could invite problems, but otoh xfiles are
never installed in the fd table so userspace should never get access
anyway.

--D

> -
>  	lockdep_set_class(&inode->i_rwsem, &xfile_i_mutex_key);
>  
>  	trace_xfile_create(xf);
> -- 
> 2.39.2
> 
>