On Wed 05-10-16 14:42:54, Jan Kara wrote:
> Currently we just silently ignore flags that we don't understand (or
> that cannot be manipulated) through EXT4_IOC_SETFLAGS and
> EXT4_IOC_FSSETXATTR ioctls. This makes it problematic for the unused
> flags to be used in future (some app may be inadvertedly setting them
> and we won't notice until the flag gets used). Also this is inconsistent
> with other filesystems like XFS or BTRFS which return EOPNOTSUPP when
> they see a flag they cannot set.
>
> ext4 has the additional problem that there are flags which are returned
> by EXT4_IOC_GETFLAGS ioctl but which cannot be modified via
> EXT4_IOC_SETFLAGS. So we have to be careful to ignore value of these
> flags and not fail the ioctl when they are set (as e.g. chattr(1) passes
> flags returned from EXT4_IOC_GETFLAGS to EXT4_IOC_SETFLAGS without any
> masking and thus we'd break this utility).
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
I've found out this does not quite work because EXT4_FL_USER_MODIFIABLE
does not contain all user modifiable flags - at least EXT4_JOURNAL_FL and
EXT4_EXTENTS_FL have special handling and can be modified even if they are
not in EXT4_FL_USER_MODIFIABLE bitmask. So this patch needs more tweaking.
Honza
> ---
> fs/ext4/ext4.h | 1 +
> fs/ext4/ioctl.c | 28 +++++++++++++++++++++++-----
> 2 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index ea31931386ec..a77a15df15b0 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -395,6 +395,7 @@ struct flex_groups {
> #define EXT4_FL_USER_VISIBLE 0x304BDFFF /* User visible flags */
> #define EXT4_FL_USER_MODIFIABLE 0x204380FF /* User modifiable flags */
>
> +/* Flags we can manipulate with through EXT4_IOC_FSSETXATTR */
> #define EXT4_FL_XFLAG_VISIBLE (EXT4_SYNC_FL | \
> EXT4_IMMUTABLE_FL | \
> EXT4_APPEND_FL | \
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index 1bb7df5e4536..540bababfec1 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -412,6 +412,10 @@ static inline __u32 ext4_iflags_to_xflags(unsigned long iflags)
> return xflags;
> }
>
> +#define EXT4_SUPPORTED_FS_XFLAGS (FS_XFLAG_SYNC | FS_XFLAG_IMMUTABLE | \
> + FS_XFLAG_APPEND | FS_XFLAG_NODUMP | \
> + FS_XFLAG_NOATIME | FS_XFLAG_PROJINHERIT)
> +
> /* Transfer xflags flags to internal */
> static inline unsigned long ext4_xflags_to_iflags(__u32 xflags)
> {
> @@ -456,12 +460,22 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> if (get_user(flags, (int __user *) arg))
> return -EFAULT;
>
> + if (flags & ~EXT4_FL_USER_VISIBLE)
> + return -EOPNOTSUPP;
> + /*
> + * chattr(1) grabs flags via GETFLAGS, modifies the result and
> + * passes that to SETFLAGS. So we cannot easily make SETFLAGS
> + * more restrictive than just silently masking off visible but
> + * not settable flags as we always did.
> + */
> + flags &= EXT4_FL_USER_MODIFIABLE;
> + if (ext4_mask_flags(inode->i_mode, flags) != flags)
> + return -EOPNOTSUPP;
> +
> err = mnt_want_write_file(filp);
> if (err)
> return err;
>
> - flags = ext4_mask_flags(inode->i_mode, flags);
> -
> inode_lock(inode);
> err = ext4_ioctl_setflags(inode, flags);
> inode_unlock(inode);
> @@ -866,13 +880,17 @@ resizefs_out:
> if (!inode_owner_or_capable(inode))
> return -EACCES;
>
> + if (fa.fsx_xflags & ~EXT4_SUPPORTED_FS_XFLAGS)
> + return -EOPNOTSUPP;
> +
> + flags = ext4_xflags_to_iflags(fa.fsx_xflags);
> + if (ext4_mask_flags(inode->i_mode, flags) != flags)
> + return -EOPNOTSUPP;
> +
> err = mnt_want_write_file(filp);
> if (err)
> return err;
>
> - flags = ext4_xflags_to_iflags(fa.fsx_xflags);
> - flags = ext4_mask_flags(inode->i_mode, flags);
> -
> inode_lock(inode);
> flags = (ei->i_flags & ~EXT4_FL_XFLAG_VISIBLE) |
> (flags & EXT4_FL_XFLAG_VISIBLE);
> --
> 2.6.6
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
