The patch below does not apply to the 5.4-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 89bdfaf93d9157499c3a0d61f489df66f2dead7f Mon Sep 17 00:00:00 2001
From: Miklos Szeredi mszeredi@redhat.com Date: Mon, 14 Dec 2020 15:26:14 +0100 Subject: [PATCH] ovl: make ioctl() safe
ovl_ioctl_set_flags() does a capability check using flags, but then the real ioctl double-fetches flags and uses potentially different value.
The "Check the capability before cred override" comment misleading: user can skip this check by presenting benign flags first and then overwriting them to non-benign flags.
Just remove the cred override for now, hoping this doesn't cause a regression.
The proper solution is to create a new setxflags i_op (patches are in the works).
Xfstests don't show a regression.
Reported-by: Dmitry Vyukov dvyukov@google.com Signed-off-by: Miklos Szeredi mszeredi@redhat.com Reviewed-by: Amir Goldstein amir73il@gmail.com Fixes: dab5ca8fd9dd ("ovl: add lsattr/chattr support") Cc: stable@vger.kernel.org # v4.19
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c index efccb7c1f9bc..a1f72ac053e5 100644 --- a/fs/overlayfs/file.c +++ b/fs/overlayfs/file.c @@ -541,46 +541,31 @@ static long ovl_real_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { struct fd real; - const struct cred *old_cred; long ret;
ret = ovl_real_fdget(file, &real); if (ret) return ret;
- old_cred = ovl_override_creds(file_inode(file)->i_sb); ret = security_file_ioctl(real.file, cmd, arg); - if (!ret) + if (!ret) { + /* + * Don't override creds, since we currently can't safely check + * permissions before doing so. + */ ret = vfs_ioctl(real.file, cmd, arg); - revert_creds(old_cred); + }
fdput(real);
return ret; }
-static unsigned int ovl_iflags_to_fsflags(unsigned int iflags) -{ - unsigned int flags = 0; - - if (iflags & S_SYNC) - flags |= FS_SYNC_FL; - if (iflags & S_APPEND) - flags |= FS_APPEND_FL; - if (iflags & S_IMMUTABLE) - flags |= FS_IMMUTABLE_FL; - if (iflags & S_NOATIME) - flags |= FS_NOATIME_FL; - - return flags; -} - static long ovl_ioctl_set_flags(struct file *file, unsigned int cmd, - unsigned long arg, unsigned int flags) + unsigned long arg) { long ret; struct inode *inode = file_inode(file); - unsigned int oldflags;
if (!inode_owner_or_capable(inode)) return -EACCES; @@ -591,10 +576,13 @@ static long ovl_ioctl_set_flags(struct file *file, unsigned int cmd,
inode_lock(inode);
- /* Check the capability before cred override */ - oldflags = ovl_iflags_to_fsflags(READ_ONCE(inode->i_flags)); - ret = vfs_ioc_setflags_prepare(inode, oldflags, flags); - if (ret) + /* + * Prevent copy up if immutable and has no CAP_LINUX_IMMUTABLE + * capability. + */ + ret = -EPERM; + if (!ovl_has_upperdata(inode) && IS_IMMUTABLE(inode) && + !capable(CAP_LINUX_IMMUTABLE)) goto unlock;
ret = ovl_maybe_copy_up(file_dentry(file), O_WRONLY); @@ -613,46 +601,6 @@ static long ovl_ioctl_set_flags(struct file *file, unsigned int cmd,
}
-static long ovl_ioctl_set_fsflags(struct file *file, unsigned int cmd, - unsigned long arg) -{ - unsigned int flags; - - if (get_user(flags, (int __user *) arg)) - return -EFAULT; - - return ovl_ioctl_set_flags(file, cmd, arg, flags); -} - -static unsigned int ovl_fsxflags_to_fsflags(unsigned int xflags) -{ - unsigned int flags = 0; - - if (xflags & FS_XFLAG_SYNC) - flags |= FS_SYNC_FL; - if (xflags & FS_XFLAG_APPEND) - flags |= FS_APPEND_FL; - if (xflags & FS_XFLAG_IMMUTABLE) - flags |= FS_IMMUTABLE_FL; - if (xflags & FS_XFLAG_NOATIME) - flags |= FS_NOATIME_FL; - - return flags; -} - -static long ovl_ioctl_set_fsxflags(struct file *file, unsigned int cmd, - unsigned long arg) -{ - struct fsxattr fa; - - memset(&fa, 0, sizeof(fa)); - if (copy_from_user(&fa, (void __user *) arg, sizeof(fa))) - return -EFAULT; - - return ovl_ioctl_set_flags(file, cmd, arg, - ovl_fsxflags_to_fsflags(fa.fsx_xflags)); -} - long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { long ret; @@ -663,12 +611,9 @@ long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg) ret = ovl_real_ioctl(file, cmd, arg); break;
- case FS_IOC_SETFLAGS: - ret = ovl_ioctl_set_fsflags(file, cmd, arg); - break; - case FS_IOC_FSSETXATTR: - ret = ovl_ioctl_set_fsxflags(file, cmd, arg); + case FS_IOC_SETFLAGS: + ret = ovl_ioctl_set_flags(file, cmd, arg); break;
default: