ACK/cmnt: [SRU][UNSTABLE/EOAN/FOCAL][PATCH] UBUNTU: SAUCE: shiftfs: let userns root destroy subvolumes from other users
Kleber Souza
kleber.souza at canonical.com
Thu May 28 08:39:47 UTC 2020
On 2020-05-20 13:44, Christian Brauner wrote:
> BugLink: https://bugs.launchpad.net/bugs/1879688
>
> Stéphane reported a bug found during NorthSec that makes heavy use of
> shiftfs. When a subvolume or snapshot is created as userns root in the
> container and then chowned to another user a delete as the root user
> will fail. The reason for this is that we drop all capabilities as a
> safety measure before calling btrfs ioctls. The only workable fix I
> could think of is to retain the CAP_DAC_OVERRIDE capability for the
> BTRFS_IOC_SNAP_DESTROY ioctl. All other solutions would be way more
> invasive.
>
> Cc: Seth Forshee <seth.forshee at canonical.com>
> Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
The change looks good to me, but I'd also like to see Seth's comment
on it.
Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
Thanks,
Kleber
> ---
> fs/shiftfs.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/fs/shiftfs.c b/fs/shiftfs.c
> index 652fb67b0e0b..2636871f05f4 100644
> --- a/fs/shiftfs.c
> +++ b/fs/shiftfs.c
> @@ -1343,7 +1343,7 @@ static int shiftfs_fadvise(struct file *file, loff_t offset, loff_t len,
> return ret;
> }
>
> -static int shiftfs_override_ioctl_creds(const struct super_block *sb,
> +static int shiftfs_override_ioctl_creds(int cmd, const struct super_block *sb,
> const struct cred **oldcred,
> struct cred **newcred)
> {
> @@ -1368,6 +1368,16 @@ static int shiftfs_override_ioctl_creds(const struct super_block *sb,
> cap_clear((*newcred)->cap_inheritable);
> cap_clear((*newcred)->cap_permitted);
>
> + if (cmd == BTRFS_IOC_SNAP_DESTROY) {
> + kuid_t kuid_root = make_kuid(sb->s_user_ns, 0);
> + /*
> + * Allow the root user in the container to remove subvolumes
> + * from other users.
> + */
> + if (uid_valid(kuid_root) && uid_eq(fsuid, kuid_root))
> + cap_raise((*newcred)->cap_effective, CAP_DAC_OVERRIDE);
> + }
> +
> put_cred(override_creds(*newcred));
> return 0;
> }
> @@ -1500,7 +1510,7 @@ static long shiftfs_real_ioctl(struct file *file, unsigned int cmd,
> if (ret)
> goto out_restore;
>
> - ret = shiftfs_override_ioctl_creds(sb, &oldcred, &newcred);
> + ret = shiftfs_override_ioctl_creds(cmd, sb, &oldcred, &newcred);
> if (ret)
> goto out_fdput;
>
>
More information about the kernel-team
mailing list