Re: [PATCH] xfs: Add check for unsupported xflags

Darrick J. Wong
2020-08-31T17:25:10Z

On Mon, Aug 31, 2020 at 09:37:45PM +0800, Xiao Yang wrote:
> Current ioctl(FSSETXATTR) ignores unsupported xflags silently
> so it it not clear for user to know unsupported xflags.
> For example, use ioctl(FSSETXATTR) to set dax flag on kernel
> v4.4 which doesn't support dax flag:
> --------------------------------
> # xfs_io -f -c "chattr +x" testfile;echo $?
> 0
> # xfs_io -c "lsattr" testfile
> ----------------X testfile
> --------------------------------
>
> Add check to report unsupported info as ioctl(SETXFLAGS) does.
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
> fs/xfs/xfs_ioctl.c | 4 ++++
> include/uapi/linux/fs.h | 8 ++++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 6f22a66777cd..cfe7f20c94fe 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1439,6 +1439,10 @@ xfs_ioctl_setattr(
>
> trace_xfs_ioctl_setattr(ip);
>
> + /* Check if fsx_xflags have unsupported xflags */
> + if (fa->fsx_xflags & ~FS_XFLAG_ALL)
> + return -EOPNOTSUPP;

Shouldn't this be in vfs_ioc_fssetxattr_check, since we're checking
against all the vfs defined XFLAGS?

--D

> +
> code = xfs_ioctl_setattr_check_projid(ip, fa);
> if (code)
> return code;
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index f44eb0a04afd..31b6856f6877 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -142,6 +142,14 @@ struct fsxattr {
> #define FS_XFLAG_COWEXTSIZE 0x00010000 /* CoW extent size allocator hint */
> #define FS_XFLAG_HASATTR 0x80000000 /* no DIFLAG for this */
>
> +#define FS_XFLAG_ALL \
> + (FS_XFLAG_REALTIME | FS_XFLAG_PREALLOC | FS_XFLAG_IMMUTABLE | \
> + FS_XFLAG_APPEND | FS_XFLAG_SYNC | FS_XFLAG_NOATIME | FS_XFLAG_NODUMP | \
> + FS_XFLAG_RTINHERIT | FS_XFLAG_PROJINHERIT | FS_XFLAG_NOSYMLINKS | \
> + FS_XFLAG_EXTSIZE | FS_XFLAG_EXTSZINHERIT | FS_XFLAG_NODEFRAG | \
> + FS_XFLAG_FILESTREAM | FS_XFLAG_DAX | FS_XFLAG_COWEXTSIZE | \
> + FS_XFLAG_HASATTR)
> +
> /* the read-only stuff doesn't really belong here, but any other place is
> probably as bad and I don't want to create yet another include file. */
>
> --
> 2.25.1
>
>
>