ACK/Cmnt: [PATCH 1/1][C/D] UBUNTU: SAUCE: overlayfs: ensure mounter privileges when reading directories

[Stefan Bader]

On 19.10.18 18:44, Tyler Hicks wrote:
> From: Andy Whitcroft <apw at canonical.com>
> 
> BugLink: https://launchpad.net/bugs/1793458
> 
> When reading directory contents ensure the mounter has permissions for
> the operation over the constituent parts (lower and upper). Where we are
> in a namespace this ensures that the mounter (root in that namespace)
> has permissions over the files and directories, preventing exposure of
> protected files and directory contents.
> 
> CVE-2018-6559
> 
> Signed-off-by: Andy Whitcroft <apw at canonical.com>
> [tyhicks: make use of new upstream check in ovl_permission() for copy-ups]
> [tyhicks: make use of creator (mounter) creds hanging off the super block]
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>
> ---

From the bug report I take this was taken from Xenial and forward ported to
Bionic and Cosmic. I wonder whether we could make this more obvious in the s-o-b
area something like

(fwd-ported from commit <sha1> xenial)

-Stefan

>  fs/overlayfs/inode.c     |  5 +----
>  fs/overlayfs/overlayfs.h |  2 ++
>  fs/overlayfs/readdir.c   | 12 ++++++++++++
>  fs/overlayfs/util.c      | 13 +++++++++++++
>  4 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index ed16a898caeb..988f7bdde3de 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -217,7 +217,6 @@ int ovl_permission(struct inode *inode, int mask)
>  {
>  	struct inode *upperinode = ovl_inode_upper(inode);
>  	struct inode *realinode = upperinode ?: ovl_inode_lower(inode);
> -	const struct cred *old_cred;
>  	int err;
>  
>  	/* Careful in RCU walk mode */
> @@ -234,15 +233,13 @@ int ovl_permission(struct inode *inode, int mask)
>  	if (err)
>  		return err;
>  
> -	old_cred = ovl_override_creds(inode->i_sb);
>  	if (!upperinode &&
>  	    !special_file(realinode->i_mode) && mask & MAY_WRITE) {
>  		mask &= ~(MAY_WRITE | MAY_APPEND);
>  		/* Make sure mounter can read file for copy up later */
>  		mask |= MAY_READ;
>  	}
> -	err = inode_permission(realinode, mask);
> -	revert_creds(old_cred);
> +	err = ovl_creator_permission(inode->i_sb, realinode, mask);
>  
>  	return err;
>  }
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 31aebb429d02..62c3c080b185 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -208,6 +208,8 @@ void ovl_drop_write(struct dentry *dentry);
>  struct dentry *ovl_workdir(struct dentry *dentry);
>  const struct cred *ovl_override_creds(struct super_block *sb);
>  struct super_block *ovl_same_sb(struct super_block *sb);
> +int ovl_creator_permission(struct super_block *sb, struct inode *inode,
> +			   int mode);
>  int ovl_can_decode_fh(struct super_block *sb);
>  struct dentry *ovl_indexdir(struct super_block *sb);
>  bool ovl_index_all(struct super_block *sb);
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index cc8303a806b4..d8ec95a4069f 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -373,6 +373,12 @@ static int ovl_dir_read_merged(struct dentry *dentry, struct list_head *list,
>  		next = ovl_path_next(idx, dentry, &realpath);
>  		rdd.is_upper = ovl_dentry_upper(dentry) == realpath.dentry;
>  
> +		err = ovl_creator_permission(dentry->d_sb,
> +					     d_inode(realpath.dentry),
> +					     MAY_READ);
> +		if (err)
> +			break;
> +
>  		if (next != -1) {
>  			err = ovl_dir_read(&realpath, &rdd);
>  			if (err)
> @@ -735,6 +741,12 @@ static int ovl_iterate(struct file *file, struct dir_context *ctx)
>  		ovl_dir_reset(file);
>  
>  	if (od->is_real) {
> +		err = ovl_creator_permission(dentry->d_sb,
> +					     file_inode(od->realfile),
> +					     MAY_READ);
> +		if (err)
> +			return err;
> +
>  		/*
>  		 * If parent is merge, then need to adjust d_ino for '..', if
>  		 * dir is impure then need to adjust d_ino for copied up
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 6f1078028c66..2ca86dbb55b6 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -55,6 +55,19 @@ struct super_block *ovl_same_sb(struct super_block *sb)
>  		return NULL;
>  }
>  
> +int ovl_creator_permission(struct super_block *sb, struct inode *inode,
> +			   int mode)
> +{
> +	const struct cred *old_cred;
> +	int err = 0;
> +
> +	old_cred = ovl_override_creds(sb);
> +	err = inode_permission(inode, mode);
> +	revert_creds(old_cred);
> +
> +	return err;
> +}
> +
>  /*
>   * Check if underlying fs supports file handles and try to determine encoding
>   * type, in order to deduce maximum inode number used by fs.
> 


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