| Message ID | 20200902040601.10293-1-yangx.jy@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] xfs: Add check for unsupported xflags | expand |
On Wed, Sep 02, 2020 at 12:06:01PM +0800, Xiao Yang wrote: > Current ioctl(FSSETXATTR) ignores unsupported xflags silently > so it is 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: > -------------------------------- > 0 > ----------------X testfile > -------------------------------- > > Add check to return -EOPNOTSUPP as ext4/f2fs/btrfs does. > > Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com> Looks good to me, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/xfs_ioctl.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > index 6f22a66777cd..e188e81961bd 100644 > --- a/fs/xfs/xfs_ioctl.c > +++ b/fs/xfs/xfs_ioctl.c > @@ -1425,6 +1425,14 @@ xfs_ioctl_setattr_check_projid( > return 0; > } > > +#define XFS_SUPPORTED_FS_XFLAGS \ > + (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) > + > STATIC int > xfs_ioctl_setattr( > xfs_inode_t *ip, > @@ -1439,6 +1447,10 @@ xfs_ioctl_setattr( > > trace_xfs_ioctl_setattr(ip); > > + /* Check if fsx_xflags has unsupported xflags */ > + if (fa->fsx_xflags & ~XFS_SUPPORTED_FS_XFLAGS) > + return -EOPNOTSUPP; > + > code = xfs_ioctl_setattr_check_projid(ip, fa); > if (code) > return code; > -- > 2.25.1 > > >
On 2020/9/3 1:58, Darrick J. Wong wrote: > On Wed, Sep 02, 2020 at 12:06:01PM +0800, Xiao Yang wrote: >> Current ioctl(FSSETXATTR) ignores unsupported xflags silently >> so it is 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: >> -------------------------------- >> 0 >> ----------------X testfile >> -------------------------------- Hi Darrick, Oops, the example shows incomplete info and see the correct one: -------------------------------------------------------- # xfs_io -f -c "chattr +x" testfile;echo $? 0 # xfs_io -c "lsattr" testfile ----------------X testfile -------------------------------------------------------- I will send the v3 patch shortly. Best Regards, Xiao Yang >> Add check to return -EOPNOTSUPP as ext4/f2fs/btrfs does. >> >> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com> > Looks good to me, > Reviewed-by: Darrick J. Wong<darrick.wong@oracle.com> > > --D > >> --- >> fs/xfs/xfs_ioctl.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c >> index 6f22a66777cd..e188e81961bd 100644 >> --- a/fs/xfs/xfs_ioctl.c >> +++ b/fs/xfs/xfs_ioctl.c >> @@ -1425,6 +1425,14 @@ xfs_ioctl_setattr_check_projid( >> return 0; >> } >> >> +#define XFS_SUPPORTED_FS_XFLAGS \ >> + (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) >> + >> STATIC int >> xfs_ioctl_setattr( >> xfs_inode_t *ip, >> @@ -1439,6 +1447,10 @@ xfs_ioctl_setattr( >> >> trace_xfs_ioctl_setattr(ip); >> >> + /* Check if fsx_xflags has unsupported xflags */ >> + if (fa->fsx_xflags& ~XFS_SUPPORTED_FS_XFLAGS) >> + return -EOPNOTSUPP; >> + >> code = xfs_ioctl_setattr_check_projid(ip, fa); >> if (code) >> return code; >> -- >> 2.25.1 >> >> >> > > . >
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index 6f22a66777cd..e188e81961bd 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -1425,6 +1425,14 @@ xfs_ioctl_setattr_check_projid( return 0; } +#define XFS_SUPPORTED_FS_XFLAGS \ + (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) + STATIC int xfs_ioctl_setattr( xfs_inode_t *ip, @@ -1439,6 +1447,10 @@ xfs_ioctl_setattr( trace_xfs_ioctl_setattr(ip); + /* Check if fsx_xflags has unsupported xflags */ + if (fa->fsx_xflags & ~XFS_SUPPORTED_FS_XFLAGS) + return -EOPNOTSUPP; + code = xfs_ioctl_setattr_check_projid(ip, fa); if (code) return code;
Current ioctl(FSSETXATTR) ignores unsupported xflags silently so it is 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: -------------------------------- 0 ----------------X testfile -------------------------------- Add check to return -EOPNOTSUPP as ext4/f2fs/btrfs does. Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com> --- fs/xfs/xfs_ioctl.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)