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

[Seth Forshee]

On Thu, May 02, 2019 at 11:47:02PM +0200, Christian Brauner wrote:
> +static void shiftfs_super_force_flags(struct super_block *sb,
> +				      unsigned long lower_flags)
> +{
> +	if (lower_flags & SB_RDONLY)
> +		sb->s_flags |= SB_RDONLY;
> +
> +	if (lower_flags & SB_NOSUID)
> +		sb->s_flags |= SB_NOSUID;
> +
> +	if (lower_flags & SB_NODEV)
> +		sb->s_flags |= SB_NODEV;
> +
> +	if (lower_flags & SB_NOEXEC)
> +		sb->s_flags |= SB_NOEXEC;
> +
> +	if (lower_flags & SB_NOATIME)
> +		sb->s_flags |= SB_NOATIME;
> +
> +	if (lower_flags & SB_NODIRATIME)
> +		sb->s_flags |= SB_NODIRATIME;

It seems kind of odd to silently set rather important flags that weren't
requested rather than returning an error, though I do find precedent for
it. Seems like unexpectedly ending up with a read-only or noexec
filesystem isn't great, but I guess it's a thing the kernel does.

But what about making this more succinct, something like:

        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)
>  {
> @@ -1904,6 +1960,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 +2030,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);
>  	}

You're calling this before shiftfs_fill_super() unconditionally sets
SB_POSIXACL, which renders forcing that flag ineffective. This needs to
be fixed.

Thanks,
Seth