| Message ID | 20190611094735.7393-2-christian.brauner@ubuntu.com |
|---|---|
| State | New |
| Headers | show |
| Series | shiftfs: allow changing ro/rw for | expand |
On 11.06.19 11:47, Christian Brauner wrote: > BugLink: https://bugs.launchpad.net/bugs/1832316 > > This enables toggling between ro/rw for btrfs subvolumes under shiftfs. > > Currently, btrfs workloads employing shiftfs cause regression. > With btrfs unprivileged users can already toggle whether a subvolume > will be ro or rw. This is broken on current shiftfs as we haven't > whitelisted these ioctls(). > To prevent such regression, we need to whitelist the ioctls > BTRFS_IOC_FS_INFO, BTRFS_IOC_SUBVOL_GETFLAGS, and > BTRFS_IOC_SUBVOL_SETFLAGS. All of them should be safe for unprivileged > users. > > Cc: Seth Forshee <seth.forshee@canonical.com> > Cc: Tyler Hicks <tyhicks@canonical.com> > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > fs/shiftfs.c | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/fs/shiftfs.c b/fs/shiftfs.c > index a1dae7ea593b..49f6714e9f95 100644 > --- a/fs/shiftfs.c > +++ b/fs/shiftfs.c > @@ -1507,9 +1507,14 @@ static long shiftfs_real_ioctl(struct file *file, unsigned int cmd, > return ret; > } > > -static bool in_ioctl_whitelist(int flag) > +static bool in_ioctl_whitelist(int flag, unsigned long arg) > { > + void __user *argp = (void __user *)arg; > + u64 flags = 0; > + > switch (flag) { > + case BTRFS_IOC_FS_INFO: > + return true; > case BTRFS_IOC_SNAP_CREATE: > return true; > case BTRFS_IOC_SNAP_CREATE_V2: > @@ -1517,6 +1522,16 @@ static bool in_ioctl_whitelist(int flag) > case BTRFS_IOC_SUBVOL_CREATE: > return true; > case BTRFS_IOC_SUBVOL_CREATE_V2: > + return true; > + case BTRFS_IOC_SUBVOL_GETFLAGS: > + return true; > + case BTRFS_IOC_SUBVOL_SETFLAGS: > + if (copy_from_user(&flags, arg, sizeof(flags))) > + return false; > + > + if (flags & ~BTRFS_SUBVOL_RDONLY) > + return false; > + > return true; > case BTRFS_IOC_SNAP_DESTROY: > return true; > @@ -1536,7 +1551,7 @@ static long shiftfs_ioctl(struct file *file, unsigned int cmd, > case FS_IOC_SETFLAGS: > break; > default: > - if (!in_ioctl_whitelist(cmd) || > + if (!in_ioctl_whitelist(cmd, arg) || > !shiftfs_passthrough_ioctls(file->f_path.dentry->d_sb->s_fs_info)) > return -ENOTTY; > } > @@ -1555,7 +1570,7 @@ static long shiftfs_compat_ioctl(struct file *file, unsigned int cmd, > case FS_IOC32_SETFLAGS: > break; > default: > - if (!in_ioctl_whitelist(cmd) || > + if (!in_ioctl_whitelist(cmd, arg) || > !shiftfs_passthrough_ioctls(file->f_path.dentry->d_sb->s_fs_info)) > return -ENOIOCTLCMD; > } >
diff --git a/fs/shiftfs.c b/fs/shiftfs.c index a1dae7ea593b..49f6714e9f95 100644 --- a/fs/shiftfs.c +++ b/fs/shiftfs.c @@ -1507,9 +1507,14 @@ static long shiftfs_real_ioctl(struct file *file, unsigned int cmd, return ret; } -static bool in_ioctl_whitelist(int flag) +static bool in_ioctl_whitelist(int flag, unsigned long arg) { + void __user *argp = (void __user *)arg; + u64 flags = 0; + switch (flag) { + case BTRFS_IOC_FS_INFO: + return true; case BTRFS_IOC_SNAP_CREATE: return true; case BTRFS_IOC_SNAP_CREATE_V2: @@ -1517,6 +1522,16 @@ static bool in_ioctl_whitelist(int flag) case BTRFS_IOC_SUBVOL_CREATE: return true; case BTRFS_IOC_SUBVOL_CREATE_V2: + return true; + case BTRFS_IOC_SUBVOL_GETFLAGS: + return true; + case BTRFS_IOC_SUBVOL_SETFLAGS: + if (copy_from_user(&flags, arg, sizeof(flags))) + return false; + + if (flags & ~BTRFS_SUBVOL_RDONLY) + return false; + return true; case BTRFS_IOC_SNAP_DESTROY: return true; @@ -1536,7 +1551,7 @@ static long shiftfs_ioctl(struct file *file, unsigned int cmd, case FS_IOC_SETFLAGS: break; default: - if (!in_ioctl_whitelist(cmd) || + if (!in_ioctl_whitelist(cmd, arg) || !shiftfs_passthrough_ioctls(file->f_path.dentry->d_sb->s_fs_info)) return -ENOTTY; } @@ -1555,7 +1570,7 @@ static long shiftfs_compat_ioctl(struct file *file, unsigned int cmd, case FS_IOC32_SETFLAGS: break; default: - if (!in_ioctl_whitelist(cmd) || + if (!in_ioctl_whitelist(cmd, arg) || !shiftfs_passthrough_ioctls(file->f_path.dentry->d_sb->s_fs_info)) return -ENOIOCTLCMD; }
BugLink: https://bugs.launchpad.net/bugs/1832316 This enables toggling between ro/rw for btrfs subvolumes under shiftfs. Currently, btrfs workloads employing shiftfs cause regression. With btrfs unprivileged users can already toggle whether a subvolume will be ro or rw. This is broken on current shiftfs as we haven't whitelisted these ioctls(). To prevent such regression, we need to whitelist the ioctls BTRFS_IOC_FS_INFO, BTRFS_IOC_SUBVOL_GETFLAGS, and BTRFS_IOC_SUBVOL_SETFLAGS. All of them should be safe for unprivileged users. Cc: Seth Forshee <seth.forshee@canonical.com> Cc: Tyler Hicks <tyhicks@canonical.com> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> --- fs/shiftfs.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)