ACK: [PATCH v1][SRU][Disco] UBUNTU: SAUCE: shiftfs: lock down certain superblock flags

[Kleber Souza]

On 5/8/19 2:13 PM, Christian Brauner wrote:
> From: Christian Brauner <christian at 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 at 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 at 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;
>