APPLIED: [PATCH v1][SRU][EOAN] UBUNTU: SAUCE: overlayfs: allow with shiftfs as underlay

Kleber Souza kleber.souza at canonical.com
Wed Oct 16 16:05:10 UTC 2019


On 02.10.19 09:58, Christian Brauner wrote:
> BugLink: https://bugs.launchpad.net/bugs/1846272
> 
> In commit [1] we enabled overlayfs on top of shiftfs. This approach was
> buggy since it let to a regression for some standard overlayfs workloads
> (cf. [2]).
> In our original approach in [1] Seth and I concluded that running
> overlayfs on top of shiftfs was not possible because of the way
> overlayfs is currently opening files. The fact that it did not pass down
> the dentry of shiftfs but rather it's own caused shiftfs to be confused
> since it stashes away necessary information in d_fsdata.
> Our solution was to modify open_with_fake_path() to also take a dentry
> as an argument, then change overlayfs to pass in the shiftfs dentry
> which then would override the dentry in the passed in struct path in
> open_with_fake_path().
> However, this led to a regression for some standard overlayfs workloads
> (cf. [2]).
> After various discussions involving Seth and myself in Paris we
> concluded the reason for the regression was that we effectively created
> a struct path that was comprised of the vfsmount of the overlayfs dentry
> and the dentry of shiftfs. This is obviously broken.
> The fix is to a) not modify open_with_fake_path() and b) change
> overlayfs to do what shiftfs is doing, namely correctly setup the struct
> path such that vfsmount and dentry match and are both from shiftfs.
> Note, that overlayfs already does this for the .open method for
> directories. It just did not do it for the .open method for regular
> files leading to this issue. The reason why this hasn't been a problem
> for overlayfs so far is that it didn't allow running on top of
> filesystems that make use of d_fsdata _implicitly_ by disallowing any
> filesystem that is itself an overlay, or has revalidate methods for it's
> dentries as those usually have d_fsdata set up. Any other filesystem
> falling in this category would have suffered from the same problem.
> 
> Seth managed to trigger the regression with the following script:
>  #!/bin/bash
> 
>  utils=(bash cat)
> 
>  mkdir -p lower/proc upper work root
>  for util in ${utils[@]}; do
>  	path="$(which $util)"
>  	dir="$(dirname $path)"
>  	mkdir -p "lower/$dir"
>  	cp -v "$path" "lower/$path"
>  	libs="$(ldd $path | egrep -o '(/usr)?/lib.*\.[0-9]')"
>  	for lib in $libs; do
>  		dir="$(dirname $lib)"
>  		mkdir -p "lower/$dir"
>  		cp -v "$lib" "lower/$lib"
>  	done
>  done
> 
>  mount -t overlay -o lowerdir=lower,upperdir=upper,workdir=work nodev root
>  mount -t proc nodev root/proc
>  chroot root bash -c "cat /proc/self/maps"
>  umount root/proc
>  umount root
> 
> With the patch here applied the regression is not reproducible.
> 
> /* References */
> [1]: 37430e430a14 ("UBUNTU: SAUCE: shiftfs: enable overlayfs on shiftfs")
> [2]: https://bugs.launchpad.net/bugs/1838677
> 
> Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>

Applied to eoan/master-next branch.

Thanks,
Kleber

> ---
> /* v1 */
> unchanged
> 
> /* v0 */
> Link: https://lists.ubuntu.com/archives/kernel-team/2019-October/104185.html
> ---
>  fs/overlayfs/file.c  | 4 +++-
>  fs/overlayfs/super.c | 5 +++--
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index e235a635d9ec..895f2c5565d3 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -24,13 +24,15 @@ static char ovl_whatisit(struct inode *inode, struct inode *realinode)
>  static struct file *ovl_open_realfile(const struct file *file,
>  				      struct inode *realinode)
>  {
> +	struct path realpath;
>  	struct inode *inode = file_inode(file);
>  	struct file *realfile;
>  	const struct cred *old_cred;
>  	int flags = file->f_flags | O_NOATIME | FMODE_NONOTIFY;
>  
>  	old_cred = ovl_override_creds(inode->i_sb);
> -	realfile = open_with_fake_path(&file->f_path, flags, realinode,
> +	ovl_path_real(file->f_path.dentry, &realpath);
> +	realfile = open_with_fake_path(&realpath, flags, realinode,
>  				       current_cred());
>  	revert_creds(old_cred);
>  
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index ec3f4cc537f0..9cd3f61f449a 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -750,13 +750,14 @@ static int ovl_mount_dir(const char *name, struct path *path)
>  		ovl_unescape(tmp);
>  		err = ovl_mount_dir_noesc(tmp, path);
>  
> -		if (!err)
> -			if (ovl_dentry_remote(path->dentry)) {
> +		if (!err) {
> +			if ((path->dentry->d_sb->s_magic != SHIFTFS_MAGIC) && ovl_dentry_remote(path->dentry)) {
>  				pr_err("overlayfs: filesystem on '%s' not supported as upperdir\n",
>  				       tmp);
>  				path_put_init(path);
>  				err = -EINVAL;
>  			}
> +		}
>  		kfree(tmp);
>  	}
>  	return err;
> 




More information about the kernel-team mailing list