Re: [PATCH v2 1/4] vfs: create a generic checking function for FS_IOC_SETFLAGS
On Tue 11-06-19 17:42:58, Darrick J. Wong wrote:
> From: Darrick J. Wong
>
> 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
The patch looks good to me. You can add:
Reviewed-by: Jan Kara
Honza
> ---
> v2: fix jfs locking and remove its opencoded flags check
> ---
> 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 | 22 +++---
> 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, 108 insertions(+), 108 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 = -EPE
[PATCH v2 1/4] vfs: create a generic checking function for FS_IOC_SETFLAGS
From: Darrick J. Wong
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
---
v2: fix jfs locking and remove its opencoded flags check
---
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 | 22 +++---
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, 108 insertions(+), 108 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_U
