On 2020/9/1 1:22, Darrick J. Wong wrote:
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.
Hi Darrick,
Sorry for a typo(s/it it/it is/).
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@xxxxxxxxxxxxxx>
---
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?
Right, different filesystems support different XFLAGS so I think it is
hard to put this
check into vfs_ioc_fssetxattr_check(). For example,
1) ext4 defines EXT4_SUPPORTED_FS_XFLAGS and do the check before
vfs_ioc_fssetxattr_check():
-------------------------------------------------------------------------------
ext4/ioctl.c:
#define EXT4_SUPPORTED_FS_XFLAGS (FS_XFLAG_SYNC | FS_XFLAG_IMMUTABLE | \
FS_XFLAG_APPEND | FS_XFLAG_NODUMP | \
FS_XFLAG_NOATIME |
FS_XFLAG_PROJINHERIT | \
FS_XFLAG_DAX)
...
if (fa.fsx_xflags & ~EXT4_SUPPORTED_FS_XFLAGS)
return -EOPNOTSUPP;
...
-------------------------------------------------------------------------------
2) btrfs adds check_xflags() and calls it before vfs_ioc_fssetxattr_check():
-------------------------------------------------------------------------------
btrfs/ioctl.c:
static int check_xflags(unsigned int flags)
{
if (flags & ~(FS_XFLAG_APPEND | FS_XFLAG_IMMUTABLE |
FS_XFLAG_NOATIME |
FS_XFLAG_NODUMP | FS_XFLAG_SYNC))
return -EOPNOTSUPP;
return 0;
}
...
ret = check_xflags(fa.fsx_xflags);
if (ret)
return ret;
...
-------------------------------------------------------------------------------
Perhaps, I should rename FS_XFLAG_ALL to XFS_SUPPORTED_FS_XFLAGS and move
it into libxfs/xfs_fs.h.
Best Regards,
Xiao Yang
--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
.
