Darrick J. Wong
2019-Jun-11 04:45 UTC
[Ocfs2-devel] [PATCH 0/4] vfs: clean up SETFLAGS and FSSETXATTR option processing
Hi all, The FS_IOC_SETFLAGS and FS_IOC_FSSETXATTR ioctls were promoted from ext4 and XFS, respectively, into the VFS. However, we didn't promote any of the parameter checking code from those filesystems, which lead to a mess where each filesystem open-codes whatever parameter checks they want and the behavior across filesystems is no longer consistent. Therefore, create some generic checking functions in the VFS and remove all the open-coded pieces in each filesystem. This preserves the current behavior where a filesystem can choose to ignore fields it doesn't understand. If you're going to start using this mess, you probably ought to just pull from my git trees, which are linked below. This has been lightly tested with fstests. Enjoy! Comments and questions are, as always, welcome. --D kernel git tree: https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=file-ioctl-cleanups
Darrick J. Wong
2019-Jun-11 04:45 UTC
[Ocfs2-devel] [PATCH 1/4] vfs: create a generic checking function for FS_IOC_SETFLAGS
From: Darrick J. Wong <darrick.wong at oracle.com> Create a generic checking function for the incoming FS_IOC_SETFLAGS flag values so that we can standardize the implementations that follow ext4's flag values. Signed-off-by: Darrick J. Wong <darrick.wong at oracle.com> --- fs/btrfs/ioctl.c | 13 +++++-------- fs/efivarfs/file.c | 18 +++++++++++++----- fs/ext2/ioctl.c | 16 ++++------------ fs/ext4/ioctl.c | 13 +++---------- fs/f2fs/file.c | 7 ++++--- fs/gfs2/file.c | 42 +++++++++++++++++++++++++++++------------- fs/hfsplus/ioctl.c | 21 ++++++++++++--------- fs/inode.c | 17 +++++++++++++++++ fs/jfs/ioctl.c | 6 ++++++ fs/nilfs2/ioctl.c | 9 ++------- fs/ocfs2/ioctl.c | 13 +++---------- fs/reiserfs/ioctl.c | 10 ++++------ fs/ubifs/ioctl.c | 13 +++---------- include/linux/fs.h | 2 ++ 14 files changed, 107 insertions(+), 93 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 6dafa857bbb9..f408aa93b0cf 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -187,7 +187,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg) struct btrfs_inode *binode = BTRFS_I(inode); struct btrfs_root *root = binode->root; struct btrfs_trans_handle *trans; - unsigned int fsflags; + unsigned int fsflags, old_fsflags; int ret; const char *comp = NULL; u32 binode_flags = binode->flags; @@ -212,13 +212,10 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg) inode_lock(inode); fsflags = btrfs_mask_fsflags_for_type(inode, fsflags); - if ((fsflags ^ btrfs_inode_flags_to_fsflags(binode->flags)) & - (FS_APPEND_FL | FS_IMMUTABLE_FL)) { - if (!capable(CAP_LINUX_IMMUTABLE)) { - ret = -EPERM; - goto out_unlock; - } - } + old_fsflags = btrfs_inode_flags_to_fsflags(binode->flags); + ret = vfs_ioc_setflags_check(inode, old_fsflags, fsflags); + if (ret) + goto out_unlock; if (fsflags & FS_SYNC_FL) binode_flags |= BTRFS_INODE_SYNC; diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c index 8e568428c88b..f4f6c1bec132 100644 --- a/fs/efivarfs/file.c +++ b/fs/efivarfs/file.c @@ -110,16 +110,22 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, return size; } -static int -efivarfs_ioc_getxflags(struct file *file, void __user *arg) +static inline unsigned int efivarfs_getflags(struct inode *inode) { - struct inode *inode = file->f_mapping->host; unsigned int i_flags; unsigned int flags = 0; i_flags = inode->i_flags; if (i_flags & S_IMMUTABLE) flags |= FS_IMMUTABLE_FL; + return flags; +} + +static int +efivarfs_ioc_getxflags(struct file *file, void __user *arg) +{ + struct inode *inode = file->f_mapping->host; + unsigned int flags = efivarfs_getflags(inode); if (copy_to_user(arg, &flags, sizeof(flags))) return -EFAULT; @@ -132,6 +138,7 @@ efivarfs_ioc_setxflags(struct file *file, void __user *arg) struct inode *inode = file->f_mapping->host; unsigned int flags; unsigned int i_flags = 0; + unsigned int oldflags = efivarfs_getflags(inode); int error; if (!inode_owner_or_capable(inode)) @@ -143,8 +150,9 @@ efivarfs_ioc_setxflags(struct file *file, void __user *arg) if (flags & ~FS_IMMUTABLE_FL) return -EOPNOTSUPP; - if (!capable(CAP_LINUX_IMMUTABLE)) - return -EPERM; + error = vfs_ioc_setflags_check(inode, oldflags, flags); + if (error) + return error; if (flags & FS_IMMUTABLE_FL) i_flags |= S_IMMUTABLE; diff --git a/fs/ext2/ioctl.c b/fs/ext2/ioctl.c index 0367c0039e68..88b3b9720023 100644 --- a/fs/ext2/ioctl.c +++ b/fs/ext2/ioctl.c @@ -60,18 +60,10 @@ long ext2_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) } oldflags = ei->i_flags; - /* - * The IMMUTABLE and APPEND_ONLY flags can only be changed by - * the relevant capability. - * - * This test looks nicer. Thanks to Pauline Middelink - */ - if ((flags ^ oldflags) & (EXT2_APPEND_FL | EXT2_IMMUTABLE_FL)) { - if (!capable(CAP_LINUX_IMMUTABLE)) { - inode_unlock(inode); - ret = -EPERM; - goto setflags_out; - } + ret = vfs_ioc_setflags_check(inode, oldflags, flags); + if (ret) { + inode_unlock(inode); + goto setflags_out; } flags = flags & EXT2_FL_USER_MODIFIABLE; diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c index e486e49b31ed..5126ee351a84 100644 --- a/fs/ext4/ioctl.c +++ b/fs/ext4/ioctl.c @@ -289,16 +289,9 @@ static int ext4_ioctl_setflags(struct inode *inode, /* The JOURNAL_DATA flag is modifiable only by root */ jflag = flags & EXT4_JOURNAL_DATA_FL; - /* - * The IMMUTABLE and APPEND_ONLY flags can only be changed by - * the relevant capability. - * - * This test looks nicer. Thanks to Pauline Middelink - */ - if ((flags ^ oldflags) & (EXT4_APPEND_FL | EXT4_IMMUTABLE_FL)) { - if (!capable(CAP_LINUX_IMMUTABLE)) - goto flags_out; - } + err = vfs_ioc_setflags_check(inode, oldflags, flags); + if (err) + goto flags_out; /* * The JOURNAL_DATA flag can only be changed by diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 45b45f37d347..a969d5497e03 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -1670,6 +1670,7 @@ static int __f2fs_ioc_setflags(struct inode *inode, unsigned int flags) { struct f2fs_inode_info *fi = F2FS_I(inode); unsigned int oldflags; + int err; /* Is it quota file? Do not allow user to mess with it */ if (IS_NOQUOTA(inode)) @@ -1679,9 +1680,9 @@ static int __f2fs_ioc_setflags(struct inode *inode, unsigned int flags) oldflags = fi->i_flags; - if ((flags ^ oldflags) & (F2FS_APPEND_FL | F2FS_IMMUTABLE_FL)) - if (!capable(CAP_LINUX_IMMUTABLE)) - return -EPERM; + err = vfs_ioc_setflags_check(inode, oldflags, flags); + if (err) + return err; flags = flags & F2FS_FL_USER_MODIFIABLE; flags |= oldflags & ~F2FS_FL_USER_MODIFIABLE; diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c index d174b1f8fd08..99f53cf699c6 100644 --- a/fs/gfs2/file.c +++ b/fs/gfs2/file.c @@ -136,27 +136,36 @@ static struct { {FS_JOURNAL_DATA_FL, GFS2_DIF_JDATA | GFS2_DIF_INHERIT_JDATA}, }; +static inline u32 gfs2_gfsflags_to_fsflags(struct inode *inode, u32 gfsflags) +{ + int i; + u32 fsflags = 0; + + if (S_ISDIR(inode->i_mode)) + gfsflags &= ~GFS2_DIF_JDATA; + else + gfsflags &= ~GFS2_DIF_INHERIT_JDATA; + + for (i = 0; i < ARRAY_SIZE(fsflag_gfs2flag); i++) + if (gfsflags & fsflag_gfs2flag[i].gfsflag) + fsflags |= fsflag_gfs2flag[i].fsflag; + return fsflags; +} + static int gfs2_get_flags(struct file *filp, u32 __user *ptr) { struct inode *inode = file_inode(filp); struct gfs2_inode *ip = GFS2_I(inode); struct gfs2_holder gh; - int i, error; - u32 gfsflags, fsflags = 0; + int error; + u32 fsflags; gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, &gh); error = gfs2_glock_nq(&gh); if (error) goto out_uninit; - gfsflags = ip->i_diskflags; - if (S_ISDIR(inode->i_mode)) - gfsflags &= ~GFS2_DIF_JDATA; - else - gfsflags &= ~GFS2_DIF_INHERIT_JDATA; - for (i = 0; i < ARRAY_SIZE(fsflag_gfs2flag); i++) - if (gfsflags & fsflag_gfs2flag[i].gfsflag) - fsflags |= fsflag_gfs2flag[i].fsflag; + fsflags = gfs2_gfsflags_to_fsflags(inode, ip->i_diskflags); if (put_user(fsflags, ptr)) error = -EFAULT; @@ -200,9 +209,11 @@ void gfs2_set_inode_flags(struct inode *inode) * @filp: file pointer * @reqflags: The flags to set * @mask: Indicates which flags are valid + * @fsflags: The FS_* inode flags passed in * */ -static int do_gfs2_set_flags(struct file *filp, u32 reqflags, u32 mask) +static int do_gfs2_set_flags(struct file *filp, u32 reqflags, u32 mask, + const u32 fsflags) { struct inode *inode = file_inode(filp); struct gfs2_inode *ip = GFS2_I(inode); @@ -210,7 +221,7 @@ static int do_gfs2_set_flags(struct file *filp, u32 reqflags, u32 mask) struct buffer_head *bh; struct gfs2_holder gh; int error; - u32 new_flags, flags; + u32 new_flags, flags, oldflags; error = mnt_want_write_file(filp); if (error) @@ -220,6 +231,11 @@ static int do_gfs2_set_flags(struct file *filp, u32 reqflags, u32 mask) if (error) goto out_drop_write; + oldflags = gfs2_gfsflags_to_fsflags(inode, ip->i_diskflags); + error = vfs_ioc_setflags_check(inode, oldflags, fsflags); + if (error) + goto out; + error = -EACCES; if (!inode_owner_or_capable(inode)) goto out; @@ -308,7 +324,7 @@ static int gfs2_set_flags(struct file *filp, u32 __user *ptr) mask &= ~(GFS2_DIF_TOPDIR | GFS2_DIF_INHERIT_JDATA); } - return do_gfs2_set_flags(filp, gfsflags, mask); + return do_gfs2_set_flags(filp, gfsflags, mask, fsflags); } static int gfs2_getlabel(struct file *filp, char __user *label) diff --git a/fs/hfsplus/ioctl.c b/fs/hfsplus/ioctl.c index 5e6502ef7415..862a3c9481d7 100644 --- a/fs/hfsplus/ioctl.c +++ b/fs/hfsplus/ioctl.c @@ -57,9 +57,8 @@ static int hfsplus_ioctl_bless(struct file *file, int __user *user_flags) return 0; } -static int hfsplus_ioctl_getflags(struct file *file, int __user *user_flags) +static inline unsigned int hfsplus_getflags(struct inode *inode) { - struct inode *inode = file_inode(file); struct hfsplus_inode_info *hip = HFSPLUS_I(inode); unsigned int flags = 0; @@ -69,6 +68,13 @@ static int hfsplus_ioctl_getflags(struct file *file, int __user *user_flags) flags |= FS_APPEND_FL; if (hip->userflags & HFSPLUS_FLG_NODUMP) flags |= FS_NODUMP_FL; + return flags; +} + +static int hfsplus_ioctl_getflags(struct file *file, int __user *user_flags) +{ + struct inode *inode = file_inode(file); + unsigned int flags = hfsplus_getflags(inode); return put_user(flags, user_flags); } @@ -78,6 +84,7 @@ static int hfsplus_ioctl_setflags(struct file *file, int __user *user_flags) struct inode *inode = file_inode(file); struct hfsplus_inode_info *hip = HFSPLUS_I(inode); unsigned int flags, new_fl = 0; + unsigned int oldflags = hfsplus_getflags(inode); int err = 0; err = mnt_want_write_file(file); @@ -96,13 +103,9 @@ static int hfsplus_ioctl_setflags(struct file *file, int __user *user_flags) inode_lock(inode); - if ((flags & (FS_IMMUTABLE_FL|FS_APPEND_FL)) || - inode->i_flags & (S_IMMUTABLE|S_APPEND)) { - if (!capable(CAP_LINUX_IMMUTABLE)) { - err = -EPERM; - goto out_unlock_inode; - } - } + err = vfs_ioc_setflags_check(inode, oldflags, flags); + if (err) + goto out_unlock_inode; /* don't silently ignore unsupported ext2 flags */ if (flags & ~(FS_IMMUTABLE_FL|FS_APPEND_FL|FS_NODUMP_FL)) { diff --git a/fs/inode.c b/fs/inode.c index df6542ec3b88..0ce60b720608 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -2170,3 +2170,20 @@ struct timespec64 current_time(struct inode *inode) return timespec64_trunc(now, inode->i_sb->s_time_gran); } EXPORT_SYMBOL(current_time); + +/* Generic function to check FS_IOC_SETFLAGS values. */ +int vfs_ioc_setflags_check(struct inode *inode, int oldflags, int flags) +{ + /* + * The IMMUTABLE and APPEND_ONLY flags can only be changed by + * the relevant capability. + * + * This test looks nicer. Thanks to Pauline Middelink + */ + if ((flags ^ oldflags) & (FS_APPEND_FL | FS_IMMUTABLE_FL) && + !capable(CAP_LINUX_IMMUTABLE)) + return -EPERM; + + return 0; +} +EXPORT_SYMBOL(vfs_ioc_setflags_check); diff --git a/fs/jfs/ioctl.c b/fs/jfs/ioctl.c index ba34dae8bd9f..c8446d2cd0c7 100644 --- a/fs/jfs/ioctl.c +++ b/fs/jfs/ioctl.c @@ -98,6 +98,12 @@ long jfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) /* Lock against other parallel changes of flags */ inode_lock(inode); + oldflags = jfs_map_ext2(jfs_inode->mode2 & JFS_FL_USER_VISIBLE, + 0); + err = vfs_ioc_setflags_check(inode, oldflags, flags); + if (err) + goto setflags_out; + oldflags = jfs_inode->mode2; /* diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c index 9b96d79eea6c..0632336d2515 100644 --- a/fs/nilfs2/ioctl.c +++ b/fs/nilfs2/ioctl.c @@ -148,13 +148,8 @@ static int nilfs_ioctl_setflags(struct inode *inode, struct file *filp, oldflags = NILFS_I(inode)->i_flags; - /* - * The IMMUTABLE and APPEND_ONLY flags can only be changed by the - * relevant capability. - */ - ret = -EPERM; - if (((flags ^ oldflags) & (FS_APPEND_FL | FS_IMMUTABLE_FL)) && - !capable(CAP_LINUX_IMMUTABLE)) + ret = vfs_ioc_setflags_check(inode, oldflags, flags); + if (ret) goto out; ret = nilfs_transaction_begin(inode->i_sb, &ti, 0); diff --git a/fs/ocfs2/ioctl.c b/fs/ocfs2/ioctl.c index 994726ada857..467a2faf0305 100644 --- a/fs/ocfs2/ioctl.c +++ b/fs/ocfs2/ioctl.c @@ -106,16 +106,9 @@ static int ocfs2_set_inode_attr(struct inode *inode, unsigned flags, flags = flags & mask; flags |= oldflags & ~mask; - /* - * The IMMUTABLE and APPEND_ONLY flags can only be changed by - * the relevant capability. - */ - status = -EPERM; - if ((oldflags & OCFS2_IMMUTABLE_FL) || ((flags ^ oldflags) & - (OCFS2_APPEND_FL | OCFS2_IMMUTABLE_FL))) { - if (!capable(CAP_LINUX_IMMUTABLE)) - goto bail_unlock; - } + status = vfs_ioc_setflags_check(inode, oldflags, flags); + if (status) + goto bail_unlock; handle = ocfs2_start_trans(osb, OCFS2_INODE_UPDATE_CREDITS); if (IS_ERR(handle)) { diff --git a/fs/reiserfs/ioctl.c b/fs/reiserfs/ioctl.c index acbbaf7a0bb2..92bcb1ecd994 100644 --- a/fs/reiserfs/ioctl.c +++ b/fs/reiserfs/ioctl.c @@ -74,13 +74,11 @@ long reiserfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) err = -EPERM; goto setflags_out; } - if (((flags ^ REISERFS_I(inode)-> - i_attrs) & (REISERFS_IMMUTABLE_FL | - REISERFS_APPEND_FL)) - && !capable(CAP_LINUX_IMMUTABLE)) { - err = -EPERM; + err = vfs_ioc_setflags_check(inode, + REISERFS_I(inode)->i_attrs, + flags); + if (err) goto setflags_out; - } if ((flags & REISERFS_NOTAIL_FL) && S_ISREG(inode->i_mode)) { int result; diff --git a/fs/ubifs/ioctl.c b/fs/ubifs/ioctl.c index 4f1a397fda69..bdea836fc38b 100644 --- a/fs/ubifs/ioctl.c +++ b/fs/ubifs/ioctl.c @@ -107,18 +107,11 @@ static int setflags(struct inode *inode, int flags) if (err) return err; - /* - * The IMMUTABLE and APPEND_ONLY flags can only be changed by - * the relevant capability. - */ mutex_lock(&ui->ui_mutex); oldflags = ubifs2ioctl(ui->flags); - if ((flags ^ oldflags) & (FS_APPEND_FL | FS_IMMUTABLE_FL)) { - if (!capable(CAP_LINUX_IMMUTABLE)) { - err = -EPERM; - goto out_unlock; - } - } + err = vfs_ioc_setflags_check(inode, oldflags, flags); + if (err) + goto out_unlock; ui->flags = ioctl2ubifs(flags); ubifs_set_inode_flags(inode); diff --git a/include/linux/fs.h b/include/linux/fs.h index f7fdfe93e25d..1825d055808c 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3546,4 +3546,6 @@ static inline struct sock *io_uring_get_socket(struct file *file) } #endif +int vfs_ioc_setflags_check(struct inode *inode, int oldflags, int flags); + #endif /* _LINUX_FS_H */
Darrick J. Wong
2019-Jun-11 04:45 UTC
[Ocfs2-devel] [PATCH 2/4] vfs: create a generic checking function for FS_IOC_FSSETXATTR
From: Darrick J. Wong <darrick.wong at oracle.com> Create a generic checking function for the incoming FS_IOC_FSSETXATTR fsxattr values so that we can standardize some of the implementation behaviors. Signed-off-by: Darrick J. Wong <darrick.wong at oracle.com> --- fs/btrfs/ioctl.c | 21 +++++++++------- fs/ext4/ioctl.c | 27 ++++++++++++++------ fs/f2fs/file.c | 26 ++++++++++++++----- fs/inode.c | 17 +++++++++++++ fs/xfs/xfs_ioctl.c | 70 ++++++++++++++++++++++++++++++---------------------- include/linux/fs.h | 3 ++ 6 files changed, 111 insertions(+), 53 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index f408aa93b0cf..7ddda5b4b6a6 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -366,6 +366,13 @@ static int check_xflags(unsigned int flags) return 0; } +static void __btrfs_ioctl_fsgetxattr(struct btrfs_inode *binode, + struct fsxattr *fa) +{ + memset(fa, 0, sizeof(*fa)); + fa->fsx_xflags = btrfs_inode_flags_to_xflags(binode->flags); +} + /* * Set the xflags from the internal inode flags. The remaining items of fsxattr * are zeroed. @@ -375,8 +382,7 @@ static int btrfs_ioctl_fsgetxattr(struct file *file, void __user *arg) struct btrfs_inode *binode = BTRFS_I(file_inode(file)); struct fsxattr fa; - memset(&fa, 0, sizeof(fa)); - fa.fsx_xflags = btrfs_inode_flags_to_xflags(binode->flags); + __btrfs_ioctl_fsgetxattr(binode, &fa); if (copy_to_user(arg, &fa, sizeof(fa))) return -EFAULT; @@ -390,7 +396,7 @@ static int btrfs_ioctl_fssetxattr(struct file *file, void __user *arg) struct btrfs_inode *binode = BTRFS_I(inode); struct btrfs_root *root = binode->root; struct btrfs_trans_handle *trans; - struct fsxattr fa; + struct fsxattr fa, old_fa; unsigned old_flags; unsigned old_i_flags; int ret = 0; @@ -421,13 +427,10 @@ static int btrfs_ioctl_fssetxattr(struct file *file, void __user *arg) old_flags = binode->flags; old_i_flags = inode->i_flags; - /* We need the capabilities to change append-only or immutable inode */ - if (((old_flags & (BTRFS_INODE_APPEND | BTRFS_INODE_IMMUTABLE)) || - (fa.fsx_xflags & (FS_XFLAG_APPEND | FS_XFLAG_IMMUTABLE))) && - !capable(CAP_LINUX_IMMUTABLE)) { - ret = -EPERM; + __btrfs_ioctl_fsgetxattr(binode, &old_fa); + ret = vfs_ioc_fssetxattr_check(inode, &old_fa, &fa); + if (ret) goto out_unlock; - } if (fa.fsx_xflags & FS_XFLAG_SYNC) binode->flags |= BTRFS_INODE_SYNC; diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c index 5126ee351a84..c2f48c90ca45 100644 --- a/fs/ext4/ioctl.c +++ b/fs/ext4/ioctl.c @@ -721,6 +721,19 @@ static int ext4_ioctl_check_project(struct inode *inode, struct fsxattr *fa) return 0; } +static void ext4_fsgetxattr(struct inode *inode, struct fsxattr *fa) +{ + struct ext4_inode_info *ei = EXT4_I(inode); + + memset(fa, 0, sizeof(struct fsxattr)); + fa->fsx_xflags = ext4_iflags_to_xflags(ei->i_flags & EXT4_FL_USER_VISIBLE); + + if (ext4_has_feature_project(inode->i_sb)) { + fa->fsx_projid = (__u32)from_kprojid(&init_user_ns, + ei->i_projid); + } +} + long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { struct inode *inode = file_inode(filp); @@ -1089,13 +1102,7 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { struct fsxattr fa; - memset(&fa, 0, sizeof(struct fsxattr)); - fa.fsx_xflags = ext4_iflags_to_xflags(ei->i_flags & EXT4_FL_USER_VISIBLE); - - if (ext4_has_feature_project(inode->i_sb)) { - fa.fsx_projid = (__u32)from_kprojid(&init_user_ns, - EXT4_I(inode)->i_projid); - } + ext4_fsgetxattr(inode, &fa); if (copy_to_user((struct fsxattr __user *)arg, &fa, sizeof(fa))) @@ -1104,7 +1111,7 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) } case EXT4_IOC_FSSETXATTR: { - struct fsxattr fa; + struct fsxattr fa, old_fa; int err; if (copy_from_user(&fa, (struct fsxattr __user *)arg, @@ -1127,7 +1134,11 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) return err; inode_lock(inode); + ext4_fsgetxattr(inode, &old_fa); err = ext4_ioctl_check_project(inode, &fa); + if (err) + goto out; + err = vfs_ioc_fssetxattr_check(inode, &old_fa, &fa); if (err) goto out; flags = (ei->i_flags & ~EXT4_FL_XFLAG_VISIBLE) | diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index a969d5497e03..f707de6bd4a8 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -2773,19 +2773,26 @@ static inline unsigned long f2fs_xflags_to_iflags(__u32 xflags) return iflags; } -static int f2fs_ioc_fsgetxattr(struct file *filp, unsigned long arg) +static void __f2fs_ioc_fsgetxattr(struct inode *inode, + struct fsxattr *fa) { - struct inode *inode = file_inode(filp); struct f2fs_inode_info *fi = F2FS_I(inode); - struct fsxattr fa; - memset(&fa, 0, sizeof(struct fsxattr)); - fa.fsx_xflags = f2fs_iflags_to_xflags(fi->i_flags & + memset(fa, 0, sizeof(struct fsxattr)); + fa->fsx_xflags = f2fs_iflags_to_xflags(fi->i_flags & F2FS_FL_USER_VISIBLE); if (f2fs_sb_has_project_quota(F2FS_I_SB(inode))) - fa.fsx_projid = (__u32)from_kprojid(&init_user_ns, + fa->fsx_projid = (__u32)from_kprojid(&init_user_ns, fi->i_projid); +} + +static int f2fs_ioc_fsgetxattr(struct file *filp, unsigned long arg) +{ + struct inode *inode = file_inode(filp); + struct fsxattr fa; + + __f2fs_ioc_fsgetxattr(inode, &fa); if (copy_to_user((struct fsxattr __user *)arg, &fa, sizeof(fa))) return -EFAULT; @@ -2820,7 +2827,7 @@ static int f2fs_ioc_fssetxattr(struct file *filp, unsigned long arg) { struct inode *inode = file_inode(filp); struct f2fs_inode_info *fi = F2FS_I(inode); - struct fsxattr fa; + struct fsxattr fa, old_fa; unsigned int flags; int err; @@ -2844,6 +2851,11 @@ static int f2fs_ioc_fssetxattr(struct file *filp, unsigned long arg) inode_lock(inode); err = f2fs_ioctl_check_project(inode, &fa); + if (err) + goto out; + + __f2fs_ioc_fsgetxattr(inode, &old_fa); + err = vfs_ioc_fssetxattr_check(inode, &old_fa, &fa); if (err) goto out; flags = (fi->i_flags & ~F2FS_FL_XFLAG_VISIBLE) | diff --git a/fs/inode.c b/fs/inode.c index 0ce60b720608..026955258a47 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -2187,3 +2187,20 @@ int vfs_ioc_setflags_check(struct inode *inode, int oldflags, int flags) return 0; } EXPORT_SYMBOL(vfs_ioc_setflags_check); + +/* Generic function to check FS_IOC_FSSETXATTR values. */ +int vfs_ioc_fssetxattr_check(struct inode *inode, const struct fsxattr *old_fa, + struct fsxattr *fa) +{ + /* + * Can't modify an immutable/append-only file unless we have + * appropriate permission. + */ + if ((old_fa->fsx_xflags ^ fa->fsx_xflags) & + (FS_XFLAG_IMMUTABLE | FS_XFLAG_APPEND) && + !capable(CAP_LINUX_IMMUTABLE)) + return -EPERM; + + return 0; +} +EXPORT_SYMBOL(vfs_ioc_fssetxattr_check); diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index d7dfc13f30f5..08c24f2f55c3 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -879,37 +879,45 @@ xfs_di2lxflags( return flags; } -STATIC int -xfs_ioc_fsgetxattr( - xfs_inode_t *ip, - int attr, - void __user *arg) +static void +__xfs_ioc_fsgetxattr( + struct xfs_inode *ip, + bool attr, + struct fsxattr *fa) { - struct fsxattr fa; - - memset(&fa, 0, sizeof(struct fsxattr)); - - xfs_ilock(ip, XFS_ILOCK_SHARED); - fa.fsx_xflags = xfs_ip2xflags(ip); - fa.fsx_extsize = ip->i_d.di_extsize << ip->i_mount->m_sb.sb_blocklog; - fa.fsx_cowextsize = ip->i_d.di_cowextsize << + memset(fa, 0, sizeof(struct fsxattr)); + fa->fsx_xflags = xfs_ip2xflags(ip); + fa->fsx_extsize = ip->i_d.di_extsize << ip->i_mount->m_sb.sb_blocklog; + fa->fsx_cowextsize = ip->i_d.di_cowextsize << ip->i_mount->m_sb.sb_blocklog; - fa.fsx_projid = xfs_get_projid(ip); + fa->fsx_projid = xfs_get_projid(ip); if (attr) { if (ip->i_afp) { if (ip->i_afp->if_flags & XFS_IFEXTENTS) - fa.fsx_nextents = xfs_iext_count(ip->i_afp); + fa->fsx_nextents = xfs_iext_count(ip->i_afp); else - fa.fsx_nextents = ip->i_d.di_anextents; + fa->fsx_nextents = ip->i_d.di_anextents; } else - fa.fsx_nextents = 0; + fa->fsx_nextents = 0; } else { if (ip->i_df.if_flags & XFS_IFEXTENTS) - fa.fsx_nextents = xfs_iext_count(&ip->i_df); + fa->fsx_nextents = xfs_iext_count(&ip->i_df); else - fa.fsx_nextents = ip->i_d.di_nextents; + fa->fsx_nextents = ip->i_d.di_nextents; } +} + +STATIC int +xfs_ioc_fsgetxattr( + xfs_inode_t *ip, + int attr, + void __user *arg) +{ + struct fsxattr fa; + + xfs_ilock(ip, XFS_ILOCK_SHARED); + __xfs_ioc_fsgetxattr(ip, attr, &fa); xfs_iunlock(ip, XFS_ILOCK_SHARED); if (copy_to_user(arg, &fa, sizeof(fa))) @@ -1035,15 +1043,6 @@ xfs_ioctl_setattr_xflags( if ((fa->fsx_xflags & FS_XFLAG_DAX) && xfs_is_reflink_inode(ip)) return -EINVAL; - /* - * Can't modify an immutable/append-only file unless - * we have appropriate permission. - */ - if (((ip->i_d.di_flags & (XFS_DIFLAG_IMMUTABLE | XFS_DIFLAG_APPEND)) || - (fa->fsx_xflags & (FS_XFLAG_IMMUTABLE | FS_XFLAG_APPEND))) && - !capable(CAP_LINUX_IMMUTABLE)) - return -EPERM; - /* diflags2 only valid for v3 inodes. */ di_flags2 = xfs_flags2diflags2(ip, fa->fsx_xflags); if (di_flags2 && ip->i_d.di_version < 3) @@ -1323,6 +1322,7 @@ xfs_ioctl_setattr( xfs_inode_t *ip, struct fsxattr *fa) { + struct fsxattr old_fa; struct xfs_mount *mp = ip->i_mount; struct xfs_trans *tp; struct xfs_dquot *udqp = NULL; @@ -1370,7 +1370,6 @@ xfs_ioctl_setattr( goto error_free_dquots; } - if (XFS_IS_QUOTA_RUNNING(mp) && XFS_IS_PQUOTA_ON(mp) && xfs_get_projid(ip) != fa->fsx_projid) { code = xfs_qm_vop_chown_reserve(tp, ip, udqp, NULL, pdqp, @@ -1379,6 +1378,11 @@ xfs_ioctl_setattr( goto error_trans_cancel; } + __xfs_ioc_fsgetxattr(ip, false, &old_fa); + code = vfs_ioc_fssetxattr_check(VFS_I(ip), &old_fa, fa); + if (code) + goto error_trans_cancel; + code = xfs_ioctl_setattr_check_extsize(ip, fa); if (code) goto error_trans_cancel; @@ -1489,6 +1493,7 @@ xfs_ioc_setxflags( { struct xfs_trans *tp; struct fsxattr fa; + struct fsxattr old_fa; unsigned int flags; int join_flags = 0; int error; @@ -1524,6 +1529,13 @@ xfs_ioc_setxflags( goto out_drop_write; } + __xfs_ioc_fsgetxattr(ip, false, &old_fa); + error = vfs_ioc_fssetxattr_check(VFS_I(ip), &old_fa, &fa); + if (error) { + xfs_trans_cancel(tp); + goto out_drop_write; + } + error = xfs_ioctl_setattr_xflags(tp, ip, &fa); if (error) { xfs_trans_cancel(tp); diff --git a/include/linux/fs.h b/include/linux/fs.h index 1825d055808c..8dad3c80b611 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3548,4 +3548,7 @@ static inline struct sock *io_uring_get_socket(struct file *file) int vfs_ioc_setflags_check(struct inode *inode, int oldflags, int flags); +int vfs_ioc_fssetxattr_check(struct inode *inode, const struct fsxattr *old_fa, + struct fsxattr *fa); + #endif /* _LINUX_FS_H */
Darrick J. Wong
2019-Jun-11 04:45 UTC
[Ocfs2-devel] [PATCH 3/4] fs: teach vfs_ioc_fssetxattr_check to check project id info
From: Darrick J. Wong <darrick.wong at oracle.com> Standardize the project id checks for FSSETXATTR. Signed-off-by: Darrick J. Wong <darrick.wong at oracle.com> --- fs/ext4/ioctl.c | 27 --------------------------- fs/f2fs/file.c | 27 --------------------------- fs/inode.c | 13 +++++++++++++ fs/xfs/xfs_ioctl.c | 15 --------------- 4 files changed, 13 insertions(+), 69 deletions(-) diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c index c2f48c90ca45..6aa1df1918f7 100644 --- a/fs/ext4/ioctl.c +++ b/fs/ext4/ioctl.c @@ -697,30 +697,6 @@ static long ext4_ioctl_group_add(struct file *file, return err; } -static int ext4_ioctl_check_project(struct inode *inode, struct fsxattr *fa) -{ - /* - * Project Quota ID state is only allowed to change from within the init - * namespace. Enforce that restriction only if we are trying to change - * the quota ID state. Everything else is allowed in user namespaces. - */ - if (current_user_ns() == &init_user_ns) - return 0; - - if (__kprojid_val(EXT4_I(inode)->i_projid) != fa->fsx_projid) - return -EINVAL; - - if (ext4_test_inode_flag(inode, EXT4_INODE_PROJINHERIT)) { - if (!(fa->fsx_xflags & FS_XFLAG_PROJINHERIT)) - return -EINVAL; - } else { - if (fa->fsx_xflags & FS_XFLAG_PROJINHERIT) - return -EINVAL; - } - - return 0; -} - static void ext4_fsgetxattr(struct inode *inode, struct fsxattr *fa) { struct ext4_inode_info *ei = EXT4_I(inode); @@ -1135,9 +1111,6 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) inode_lock(inode); ext4_fsgetxattr(inode, &old_fa); - err = ext4_ioctl_check_project(inode, &fa); - if (err) - goto out; err = vfs_ioc_fssetxattr_check(inode, &old_fa, &fa); if (err) goto out; diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index f707de6bd4a8..183ed1ac60e1 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -2799,30 +2799,6 @@ static int f2fs_ioc_fsgetxattr(struct file *filp, unsigned long arg) return 0; } -static int f2fs_ioctl_check_project(struct inode *inode, struct fsxattr *fa) -{ - /* - * Project Quota ID state is only allowed to change from within the init - * namespace. Enforce that restriction only if we are trying to change - * the quota ID state. Everything else is allowed in user namespaces. - */ - if (current_user_ns() == &init_user_ns) - return 0; - - if (__kprojid_val(F2FS_I(inode)->i_projid) != fa->fsx_projid) - return -EINVAL; - - if (F2FS_I(inode)->i_flags & F2FS_PROJINHERIT_FL) { - if (!(fa->fsx_xflags & FS_XFLAG_PROJINHERIT)) - return -EINVAL; - } else { - if (fa->fsx_xflags & FS_XFLAG_PROJINHERIT) - return -EINVAL; - } - - return 0; -} - static int f2fs_ioc_fssetxattr(struct file *filp, unsigned long arg) { struct inode *inode = file_inode(filp); @@ -2850,9 +2826,6 @@ static int f2fs_ioc_fssetxattr(struct file *filp, unsigned long arg) return err; inode_lock(inode); - err = f2fs_ioctl_check_project(inode, &fa); - if (err) - goto out; __f2fs_ioc_fsgetxattr(inode, &old_fa); err = vfs_ioc_fssetxattr_check(inode, &old_fa, &fa); diff --git a/fs/inode.c b/fs/inode.c index 026955258a47..40ecd3a6a188 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -2201,6 +2201,19 @@ int vfs_ioc_fssetxattr_check(struct inode *inode, const struct fsxattr *old_fa, !capable(CAP_LINUX_IMMUTABLE)) return -EPERM; + /* + * Project Quota ID state is only allowed to change from within the init + * namespace. Enforce that restriction only if we are trying to change + * the quota ID state. Everything else is allowed in user namespaces. + */ + if (current_user_ns() != &init_user_ns) { + if (old_fa->fsx_projid != fa->fsx_projid) + return -EINVAL; + if ((old_fa->fsx_xflags ^ fa->fsx_xflags) & + FS_XFLAG_PROJINHERIT) + return -EINVAL; + } + return 0; } EXPORT_SYMBOL(vfs_ioc_fssetxattr_check); diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index 08c24f2f55c3..82961de98900 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -1299,21 +1299,6 @@ xfs_ioctl_setattr_check_projid( if (fa->fsx_projid > (uint16_t)-1 && !xfs_sb_version_hasprojid32bit(&ip->i_mount->m_sb)) return -EINVAL; - - /* - * Project Quota ID state is only allowed to change from within the init - * namespace. Enforce that restriction only if we are trying to change - * the quota ID state. Everything else is allowed in user namespaces. - */ - if (current_user_ns() == &init_user_ns) - return 0; - - if (xfs_get_projid(ip) != fa->fsx_projid) - return -EINVAL; - if ((fa->fsx_xflags & FS_XFLAG_PROJINHERIT) !- (ip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)) - return -EINVAL; - return 0; }
Darrick J. Wong
2019-Jun-11 04:46 UTC
[Ocfs2-devel] [PATCH 4/4] vfs: teach vfs_ioc_fssetxattr_check to check extent size hints
From: Darrick J. Wong <darrick.wong at oracle.com> Move the extent size hint checks that aren't xfs-specific to the vfs. Signed-off-by: Darrick J. Wong <darrick.wong at oracle.com> --- fs/inode.c | 18 +++++++++++++ fs/xfs/xfs_ioctl.c | 70 ++++++++++++++++++++++------------------------------ 2 files changed, 47 insertions(+), 41 deletions(-) diff --git a/fs/inode.c b/fs/inode.c index 40ecd3a6a188..a3757051fd55 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -2214,6 +2214,24 @@ int vfs_ioc_fssetxattr_check(struct inode *inode, const struct fsxattr *old_fa, return -EINVAL; } + /* Check extent size hints. */ + if ((fa->fsx_xflags & FS_XFLAG_EXTSIZE) && !S_ISREG(inode->i_mode)) + return -EINVAL; + + if ((fa->fsx_xflags & FS_XFLAG_EXTSZINHERIT) && + !S_ISDIR(inode->i_mode)) + return -EINVAL; + + if ((fa->fsx_xflags & FS_XFLAG_COWEXTSIZE) && + !S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode)) + return -EINVAL; + + /* Extent size hints of zero turn off the flags. */ + if (fa->fsx_extsize == 0) + fa->fsx_xflags &= ~(FS_XFLAG_EXTSIZE | FS_XFLAG_EXTSZINHERIT); + if (fa->fsx_cowextsize == 0) + fa->fsx_xflags &= ~FS_XFLAG_COWEXTSIZE; + return 0; } EXPORT_SYMBOL(vfs_ioc_fssetxattr_check); diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index 82961de98900..b494e7e881e3 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -1201,39 +1201,31 @@ xfs_ioctl_setattr_check_extsize( struct fsxattr *fa) { struct xfs_mount *mp = ip->i_mount; - - if ((fa->fsx_xflags & FS_XFLAG_EXTSIZE) && !S_ISREG(VFS_I(ip)->i_mode)) - return -EINVAL; - - if ((fa->fsx_xflags & FS_XFLAG_EXTSZINHERIT) && - !S_ISDIR(VFS_I(ip)->i_mode)) - return -EINVAL; + xfs_extlen_t size; + xfs_fsblock_t extsize_fsb; if (S_ISREG(VFS_I(ip)->i_mode) && ip->i_d.di_nextents && ((ip->i_d.di_extsize << mp->m_sb.sb_blocklog) != fa->fsx_extsize)) return -EINVAL; - if (fa->fsx_extsize != 0) { - xfs_extlen_t size; - xfs_fsblock_t extsize_fsb; - - extsize_fsb = XFS_B_TO_FSB(mp, fa->fsx_extsize); - if (extsize_fsb > MAXEXTLEN) - return -EINVAL; + if (fa->fsx_extsize == 0) + return 0; - if (XFS_IS_REALTIME_INODE(ip) || - (fa->fsx_xflags & FS_XFLAG_REALTIME)) { - size = mp->m_sb.sb_rextsize << mp->m_sb.sb_blocklog; - } else { - size = mp->m_sb.sb_blocksize; - if (extsize_fsb > mp->m_sb.sb_agblocks / 2) - return -EINVAL; - } + extsize_fsb = XFS_B_TO_FSB(mp, fa->fsx_extsize); + if (extsize_fsb > MAXEXTLEN) + return -EINVAL; - if (fa->fsx_extsize % size) + if (XFS_IS_REALTIME_INODE(ip) || + (fa->fsx_xflags & FS_XFLAG_REALTIME)) { + size = mp->m_sb.sb_rextsize << mp->m_sb.sb_blocklog; + } else { + size = mp->m_sb.sb_blocksize; + if (extsize_fsb > mp->m_sb.sb_agblocks / 2) return -EINVAL; - } else - fa->fsx_xflags &= ~(FS_XFLAG_EXTSIZE | FS_XFLAG_EXTSZINHERIT); + } + + if (fa->fsx_extsize % size) + return -EINVAL; return 0; } @@ -1259,6 +1251,8 @@ xfs_ioctl_setattr_check_cowextsize( struct fsxattr *fa) { struct xfs_mount *mp = ip->i_mount; + xfs_extlen_t size; + xfs_fsblock_t cowextsize_fsb; if (!(fa->fsx_xflags & FS_XFLAG_COWEXTSIZE)) return 0; @@ -1267,25 +1261,19 @@ xfs_ioctl_setattr_check_cowextsize( ip->i_d.di_version != 3) return -EINVAL; - if (!S_ISREG(VFS_I(ip)->i_mode) && !S_ISDIR(VFS_I(ip)->i_mode)) - return -EINVAL; - - if (fa->fsx_cowextsize != 0) { - xfs_extlen_t size; - xfs_fsblock_t cowextsize_fsb; + if (fa->fsx_cowextsize == 0) + return 0; - cowextsize_fsb = XFS_B_TO_FSB(mp, fa->fsx_cowextsize); - if (cowextsize_fsb > MAXEXTLEN) - return -EINVAL; + cowextsize_fsb = XFS_B_TO_FSB(mp, fa->fsx_cowextsize); + if (cowextsize_fsb > MAXEXTLEN) + return -EINVAL; - size = mp->m_sb.sb_blocksize; - if (cowextsize_fsb > mp->m_sb.sb_agblocks / 2) - return -EINVAL; + size = mp->m_sb.sb_blocksize; + if (cowextsize_fsb > mp->m_sb.sb_agblocks / 2) + return -EINVAL; - if (fa->fsx_cowextsize % size) - return -EINVAL; - } else - fa->fsx_xflags &= ~FS_XFLAG_COWEXTSIZE; + if (fa->fsx_cowextsize % size) + return -EINVAL; return 0; }