| Message ID | 20190508121314.23664-1-christian.brauner@ubuntu.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,SRU,Disco] UBUNTU: SAUCE: shiftfs: lock down certain superblock flags | expand |
On Wed, May 08, 2019 at 02:13:14PM +0200, Christian Brauner wrote: > From: Christian Brauner <christian@brauner.io> > > BugLink: https://bugs.launchpad.net/bugs/1827122 > > This locks down various superblock flags to prevent userns-root from > remounting a superblock with less restrictive options than the original > mark or underlay mount. > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Looks good to me. Acked-by: Seth Forshee <seth.forshee@canonical.com>
On Tue, May 14, 2019 at 07:08:52AM -0500, Seth Forshee wrote: > On Wed, May 08, 2019 at 02:13:14PM +0200, Christian Brauner wrote: > > From: Christian Brauner <christian@brauner.io> > > > > BugLink: https://bugs.launchpad.net/bugs/1827122 > > > > This locks down various superblock flags to prevent userns-root from > > remounting a superblock with less restrictive options than the original > > mark or underlay mount. > > > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> > > Looks good to me. > > Acked-by: Seth Forshee <seth.forshee@canonical.com> Oh, and also applied to unstable. Thanks!
On Tue, May 14, 2019 at 07:28:48AM -0500, Seth Forshee wrote: > On Tue, May 14, 2019 at 07:08:52AM -0500, Seth Forshee wrote: > > On Wed, May 08, 2019 at 02:13:14PM +0200, Christian Brauner wrote: > > > From: Christian Brauner <christian@brauner.io> > > > > > > BugLink: https://bugs.launchpad.net/bugs/1827122 > > > > > > This locks down various superblock flags to prevent userns-root from > > > remounting a superblock with less restrictive options than the original > > > mark or underlay mount. > > > > > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> > > > > Looks good to me. > > > > Acked-by: Seth Forshee <seth.forshee@canonical.com> > > Oh, and also applied to unstable. Thanks! Sweet! Thank you, Seth! :) Christian
On 5/8/19 2:13 PM, Christian Brauner wrote: > From: Christian Brauner <christian@brauner.io> > > BugLink: https://bugs.launchpad.net/bugs/1827122 > > This locks down various superblock flags to prevent userns-root from > remounting a superblock with less restrictive options than the original > mark or underlay mount. > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> The code looks good. I lack the knowledge to tell if it really does what it's intended, but trusting on Seth's ACK on its functionality. Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > fs/shiftfs.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 46 insertions(+), 1 deletion(-) > > diff --git a/fs/shiftfs.c b/fs/shiftfs.c > index 9771165d1ce0..a1dae7ea593b 100644 > --- a/fs/shiftfs.c > +++ b/fs/shiftfs.c > @@ -1808,6 +1808,33 @@ static inline bool passthrough_is_subset(int old_flags, int new_flags) > return true; > } > > +static int shiftfs_super_check_flags(unsigned long old_flags, > + unsigned long new_flags) > +{ > + if ((old_flags & SB_RDONLY) && !(new_flags & SB_RDONLY)) > + return -EPERM; > + > + if ((old_flags & SB_NOSUID) && !(new_flags & SB_NOSUID)) > + return -EPERM; > + > + if ((old_flags & SB_NODEV) && !(new_flags & SB_NODEV)) > + return -EPERM; > + > + if ((old_flags & SB_NOEXEC) && !(new_flags & SB_NOEXEC)) > + return -EPERM; > + > + if ((old_flags & SB_NOATIME) && !(new_flags & SB_NOATIME)) > + return -EPERM; > + > + if ((old_flags & SB_NODIRATIME) && !(new_flags & SB_NODIRATIME)) > + return -EPERM; > + > + if (!(old_flags & SB_POSIXACL) && (new_flags & SB_POSIXACL)) > + return -EPERM; > + > + return 0; > +} > + > static int shiftfs_remount(struct super_block *sb, int *flags, char *data) > { > int err; > @@ -1818,6 +1845,10 @@ static int shiftfs_remount(struct super_block *sb, int *flags, char *data) > if (err) > return err; > > + err = shiftfs_super_check_flags(sb->s_flags, *flags); > + if (err) > + return err; > + > /* Mark mount option cannot be changed. */ > if (info->mark || (info->mark != new.mark)) > return -EPERM; > @@ -1847,6 +1878,16 @@ struct shiftfs_data { > const char *path; > }; > > +static void shiftfs_super_force_flags(struct super_block *sb, > + unsigned long lower_flags) > +{ > + sb->s_flags |= lower_flags & (SB_RDONLY | SB_NOSUID | SB_NODEV | > + SB_NOEXEC | SB_NOATIME | SB_NODIRATIME); > + > + if (!(lower_flags & SB_POSIXACL)) > + sb->s_flags &= ~SB_POSIXACL; > +} > + > static int shiftfs_fill_super(struct super_block *sb, void *raw_data, > int silent) > { > @@ -1888,6 +1929,8 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data, > goto out_put_path; > } > > + sb->s_flags |= SB_POSIXACL; > + > if (sbinfo->mark) { > struct super_block *lower_sb = path.mnt->mnt_sb; > > @@ -1904,6 +1947,8 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data, > */ > sb->s_iflags = SB_I_NOEXEC; > > + shiftfs_super_force_flags(sb, lower_sb->s_flags); > + > /* > * Handle nesting of shiftfs mounts by referring this mark > * mount back to the original mark mount. This is more > @@ -1972,6 +2017,7 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data, > * passthrough settings. > */ > sbinfo->passthrough_mark = sbinfo_mp->passthrough; > + shiftfs_super_force_flags(sb, path.mnt->mnt_sb->s_flags); > } > > sb->s_stack_depth = dentry->d_sb->s_stack_depth + 1; > @@ -1995,7 +2041,6 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data, > sb->s_op = &shiftfs_super_ops; > sb->s_xattr = shiftfs_xattr_handlers; > sb->s_d_op = &shiftfs_dentry_ops; > - sb->s_flags |= SB_POSIXACL; > sb->s_root = d_make_root(inode); > if (!sb->s_root) { > err = -ENOMEM; >
On 5/8/19 2:13 PM, Christian Brauner wrote: > From: Christian Brauner <christian@brauner.io> > > BugLink: https://bugs.launchpad.net/bugs/1827122 > > This locks down various superblock flags to prevent userns-root from > remounting a superblock with less restrictive options than the original > mark or underlay mount. > > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> > --- > fs/shiftfs.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 46 insertions(+), 1 deletion(-) > > diff --git a/fs/shiftfs.c b/fs/shiftfs.c > index 9771165d1ce0..a1dae7ea593b 100644 > --- a/fs/shiftfs.c > +++ b/fs/shiftfs.c > @@ -1808,6 +1808,33 @@ static inline bool passthrough_is_subset(int old_flags, int new_flags) > return true; > } > > +static int shiftfs_super_check_flags(unsigned long old_flags, > + unsigned long new_flags) > +{ > + if ((old_flags & SB_RDONLY) && !(new_flags & SB_RDONLY)) > + return -EPERM; > + > + if ((old_flags & SB_NOSUID) && !(new_flags & SB_NOSUID)) > + return -EPERM; > + > + if ((old_flags & SB_NODEV) && !(new_flags & SB_NODEV)) > + return -EPERM; > + > + if ((old_flags & SB_NOEXEC) && !(new_flags & SB_NOEXEC)) > + return -EPERM; > + > + if ((old_flags & SB_NOATIME) && !(new_flags & SB_NOATIME)) > + return -EPERM; > + > + if ((old_flags & SB_NODIRATIME) && !(new_flags & SB_NODIRATIME)) > + return -EPERM; > + > + if (!(old_flags & SB_POSIXACL) && (new_flags & SB_POSIXACL)) > + return -EPERM; > + > + return 0; > +} > + > static int shiftfs_remount(struct super_block *sb, int *flags, char *data) > { > int err; > @@ -1818,6 +1845,10 @@ static int shiftfs_remount(struct super_block *sb, int *flags, char *data) > if (err) > return err; > > + err = shiftfs_super_check_flags(sb->s_flags, *flags); > + if (err) > + return err; > + > /* Mark mount option cannot be changed. */ > if (info->mark || (info->mark != new.mark)) > return -EPERM; > @@ -1847,6 +1878,16 @@ struct shiftfs_data { > const char *path; > }; > > +static void shiftfs_super_force_flags(struct super_block *sb, > + unsigned long lower_flags) > +{ > + sb->s_flags |= lower_flags & (SB_RDONLY | SB_NOSUID | SB_NODEV | > + SB_NOEXEC | SB_NOATIME | SB_NODIRATIME); > + > + if (!(lower_flags & SB_POSIXACL)) > + sb->s_flags &= ~SB_POSIXACL; > +} > + > static int shiftfs_fill_super(struct super_block *sb, void *raw_data, > int silent) > { > @@ -1888,6 +1929,8 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data, > goto out_put_path; > } > > + sb->s_flags |= SB_POSIXACL; > + > if (sbinfo->mark) { > struct super_block *lower_sb = path.mnt->mnt_sb; > > @@ -1904,6 +1947,8 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data, > */ > sb->s_iflags = SB_I_NOEXEC; > > + shiftfs_super_force_flags(sb, lower_sb->s_flags); > + > /* > * Handle nesting of shiftfs mounts by referring this mark > * mount back to the original mark mount. This is more > @@ -1972,6 +2017,7 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data, > * passthrough settings. > */ > sbinfo->passthrough_mark = sbinfo_mp->passthrough; > + shiftfs_super_force_flags(sb, path.mnt->mnt_sb->s_flags); > } > > sb->s_stack_depth = dentry->d_sb->s_stack_depth + 1; > @@ -1995,7 +2041,6 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data, > sb->s_op = &shiftfs_super_ops; > sb->s_xattr = shiftfs_xattr_handlers; > sb->s_d_op = &shiftfs_dentry_ops; > - sb->s_flags |= SB_POSIXACL; > sb->s_root = d_make_root(inode); > if (!sb->s_root) { > err = -ENOMEM; > Applied to disco/master-next branch. Thanks, Kleber
diff --git a/fs/shiftfs.c b/fs/shiftfs.c index 9771165d1ce0..a1dae7ea593b 100644 --- a/fs/shiftfs.c +++ b/fs/shiftfs.c @@ -1808,6 +1808,33 @@ static inline bool passthrough_is_subset(int old_flags, int new_flags) return true; } +static int shiftfs_super_check_flags(unsigned long old_flags, + unsigned long new_flags) +{ + if ((old_flags & SB_RDONLY) && !(new_flags & SB_RDONLY)) + return -EPERM; + + if ((old_flags & SB_NOSUID) && !(new_flags & SB_NOSUID)) + return -EPERM; + + if ((old_flags & SB_NODEV) && !(new_flags & SB_NODEV)) + return -EPERM; + + if ((old_flags & SB_NOEXEC) && !(new_flags & SB_NOEXEC)) + return -EPERM; + + if ((old_flags & SB_NOATIME) && !(new_flags & SB_NOATIME)) + return -EPERM; + + if ((old_flags & SB_NODIRATIME) && !(new_flags & SB_NODIRATIME)) + return -EPERM; + + if (!(old_flags & SB_POSIXACL) && (new_flags & SB_POSIXACL)) + return -EPERM; + + return 0; +} + static int shiftfs_remount(struct super_block *sb, int *flags, char *data) { int err; @@ -1818,6 +1845,10 @@ static int shiftfs_remount(struct super_block *sb, int *flags, char *data) if (err) return err; + err = shiftfs_super_check_flags(sb->s_flags, *flags); + if (err) + return err; + /* Mark mount option cannot be changed. */ if (info->mark || (info->mark != new.mark)) return -EPERM; @@ -1847,6 +1878,16 @@ struct shiftfs_data { const char *path; }; +static void shiftfs_super_force_flags(struct super_block *sb, + unsigned long lower_flags) +{ + sb->s_flags |= lower_flags & (SB_RDONLY | SB_NOSUID | SB_NODEV | + SB_NOEXEC | SB_NOATIME | SB_NODIRATIME); + + if (!(lower_flags & SB_POSIXACL)) + sb->s_flags &= ~SB_POSIXACL; +} + static int shiftfs_fill_super(struct super_block *sb, void *raw_data, int silent) { @@ -1888,6 +1929,8 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data, goto out_put_path; } + sb->s_flags |= SB_POSIXACL; + if (sbinfo->mark) { struct super_block *lower_sb = path.mnt->mnt_sb; @@ -1904,6 +1947,8 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data, */ sb->s_iflags = SB_I_NOEXEC; + shiftfs_super_force_flags(sb, lower_sb->s_flags); + /* * Handle nesting of shiftfs mounts by referring this mark * mount back to the original mark mount. This is more @@ -1972,6 +2017,7 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data, * passthrough settings. */ sbinfo->passthrough_mark = sbinfo_mp->passthrough; + shiftfs_super_force_flags(sb, path.mnt->mnt_sb->s_flags); } sb->s_stack_depth = dentry->d_sb->s_stack_depth + 1; @@ -1995,7 +2041,6 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data, sb->s_op = &shiftfs_super_ops; sb->s_xattr = shiftfs_xattr_handlers; sb->s_d_op = &shiftfs_dentry_ops; - sb->s_flags |= SB_POSIXACL; sb->s_root = d_make_root(inode); if (!sb->s_root) { err = -ENOMEM;