ACK: [PATCH][SRU][Disco] UBUNTU: SAUCE: shiftfs: prevent use-after-free when verifying mount options

[Stefan Bader]

On 15.04.19 15:21, Christian Brauner wrote:
> BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1824735
> 
> Copy up the passthrough mount settings of the mark mount point to the
> shiftfs overlay.
> 
> Before this commit we used to keep a reference to the shiftfs mark
> mount's shiftfs_super_info which was stashed in the superblock of the
> mark mount. The problem is that we only take a reference to the mount of
> the underlay, i.e. the filesystem that is *under* the shiftfs mark
> mount. This means when someone performs a shiftfs mark mount, then a
> shiftfs overlay mount and then immediately unmounts the shiftfs mark
> mount we muck with invalid memory since shiftfs_put_super might have
> already been called freeing that memory.
> 
> Another solution would be to start reference counting. But this would be
> overkill. We only care about the passthrough mount option of the mark
> mount. And we only need it to verify that on remount the new passthrough
> options of the shiftfs overlay are a subset of the mark mount's
> passthrough options. In other scenarios we don't care. So copying up is
> good enough and also only needs to happen once on mount, i.e. when a new
> superblock is created and the .fill_super method is called.
> 
> Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> ---
>  fs/shiftfs.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/shiftfs.c b/fs/shiftfs.c
> index 8e064756ea0c..4c8a6ec2a617 100644
> --- a/fs/shiftfs.c
> +++ b/fs/shiftfs.c
> @@ -28,7 +28,7 @@ struct shiftfs_super_info {
>  	const struct cred *creator_cred;
>  	bool mark;
>  	unsigned int passthrough;
> -	struct shiftfs_super_info *info_mark;
> +	unsigned int passthrough_mark;
>  };
>  
>  struct shiftfs_file_info {
> @@ -52,10 +52,6 @@ static inline bool shiftfs_passthrough_ioctls(struct shiftfs_super_info *info)
>  	if (!(info->passthrough & SHIFTFS_PASSTHROUGH_IOCTL))
>  		return false;
>  
> -	if (info->info_mark &&
> -	    !(info->info_mark->passthrough & SHIFTFS_PASSTHROUGH_IOCTL))
> -		return false;
> -
>  	return true;
>  }
>  
> @@ -64,10 +60,6 @@ static inline bool shiftfs_passthrough_statfs(struct shiftfs_super_info *info)
>  	if (!(info->passthrough & SHIFTFS_PASSTHROUGH_STAT))
>  		return false;
>  
> -	if (info->info_mark &&
> -	    !(info->info_mark->passthrough & SHIFTFS_PASSTHROUGH_STAT))
> -		return false;
> -
>  	return true;
>  }
>  
> @@ -1824,7 +1816,7 @@ static int shiftfs_remount(struct super_block *sb, int *flags, char *data)
>  
>  	if (info->passthrough != new.passthrough) {
>  		/* Don't allow exceeding passthrough options of mark mount. */
> -		if (!passthrough_is_subset(info->info_mark->passthrough,
> +		if (!passthrough_is_subset(info->passthrough_mark,
>  					   info->passthrough))
>  			return -EPERM;
>  
> @@ -1926,9 +1918,19 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data,
>  
>  			sbinfo->mnt = mntget(sbinfo_mp->mnt);
>  			dentry = dget(path.dentry->d_fsdata);
> +			/*
> +			 * Copy up the passthrough mount options from the
> +			 * parent mark mountpoint.
> +			 */
> +			sbinfo->passthrough_mark = sbinfo_mp->passthrough_mark;
>  		} else {
>  			sbinfo->mnt = mntget(path.mnt);
>  			dentry = dget(path.dentry);
> +			/*
> +			 * For a new mark passthrough_mark and passthrough
> +			 * are identical.
> +			 */
> +			sbinfo->passthrough_mark = sbinfo->passthrough;
>  		}
>  
>  		sbinfo->creator_cred = prepare_creds();
> @@ -1956,7 +1958,12 @@ static int shiftfs_fill_super(struct super_block *sb, void *raw_data,
>  		sbinfo->mnt = mntget(sbinfo_mp->mnt);
>  		sbinfo->creator_cred = get_cred(sbinfo_mp->creator_cred);
>  		dentry = dget(path.dentry->d_fsdata);
> -		sbinfo->info_mark = sbinfo_mp;
> +		/*
> +		 * Copy up passthrough settings from mark mountpoint so we can
> +		 * verify when the overlay wants to remount with different
> +		 * passthrough settings.
> +		 */
> +		sbinfo->passthrough_mark = sbinfo_mp->passthrough;
>  	}
>  
>  	sb->s_stack_depth = dentry->d_sb->s_stack_depth + 1;
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20190415/5ec54610/attachment.sig>