[2/2,SRU,Disco] UBUNTU: SAUCE: overlayfs: allow with shiftfs as underlay
diff mbox series

Message ID 20191001204228.4846-2-christian.brauner@ubuntu.com
State New
Headers show
Series
  • Untitled series #133636
Related show

Commit Message

Christian Brauner Oct. 1, 2019, 8:42 p.m. UTC
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@ubuntu.com>
---
 fs/overlayfs/file.c  | 4 +++-
 fs/overlayfs/super.c | 5 +++--
 2 files changed, 6 insertions(+), 3 deletions(-)

Patch
diff mbox series

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 75d8d00fa087..71bf12eb70e6 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -27,13 +27,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 3bd48da819c2..b4fb0c402216 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -753,13 +753,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;