Re: [PATCH v6 5/6] fs: prepare for extending file_get/setattr()

[Jan Kara]

On Mon 30-06-25 18:20:15, Andrey Albershteyn wrote:
> From: Amir Goldstein <amir73il@xxxxxxxxx>
> 
> We intend to add support for more xflags to selective filesystems and
> We cannot rely on copy_struct_from_user() to detect this extension.
> 
> In preparation of extending the API, do not allow setting xflags unknown
> by this kernel version.
> 
> Also do not pass the read-only flags and read-only field fsx_nextents to
> filesystem.
> 
> These changes should not affect existing chattr programs that use the
> ioctl to get fsxattr before setting the new values.
> 
> Link: https://lore.kernel.org/linux-fsdevel/20250216164029.20673-4-pali@xxxxxxxxxx/
> Cc: Pali Rohár <pali@xxxxxxxxxx>
> Cc: Andrey Albershteyn <aalbersh@xxxxxxxxxx>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> Signed-off-by: Andrey Albershteyn <aalbersh@xxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

I'd just note that:

> @@ -118,11 +119,16 @@ static int copy_fsxattr_from_user(struct fileattr *fa,
>  				  struct fsxattr __user *ufa)
>  {
>  	struct fsxattr xfa;
> +	__u32 mask = FS_XFLAGS_MASK;
>  
>  	if (copy_from_user(&xfa, ufa, sizeof(xfa)))
>  		return -EFAULT;
>  
> +	if (xfa.fsx_xflags & ~mask)
> +		return -EINVAL;
> +
>  	fileattr_fill_xflags(fa, xfa.fsx_xflags);
> +	fa->fsx_xflags &= ~FS_XFLAG_RDONLY_MASK;

This means that the two flags in FS_XFLAG_RDONLY_MASK cannot easily become
writeable in the future due to this. I think it is a sensible compromise
but I wanted to mention it.

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR