Skip to contentSkip to conversation

ovl: fix missing override creds in link of a metacopy upper

PATCH In review Amir Goldstein 3 participants 3 messages Nov 14, 2018 Overlay filesystem

lore
Share & tools
lore

Amir Goldstein

author maintainer
#0 lore raw

PATCH 1/1 ovl: fix missing override creds in link of a metacopy upper

Theodore Ts'o reported a v4.19 regression with docker-dropbox:
https://marc.info/?l=linux-fsdevel&m=154070089431116&w=2

"I was rebuilding my dropbox Docker container, and it failed in 4.19
with the following error:
...
dpkg: error: error creating new backup file \
'/var/lib/dpkg/status-old': Invalid cross-device link"

The problem did not reproduce with metacopy feature disabled. The error was caused by insufficient credentials to set "trusted.overlay.redirect" xattr on link of a metacopy file.

Reproducer:

echo Y > /sys/module/overlay/parameters/redirect_dir echo Y > /sys/module/overlay/parameters/metacopy cd /tmp
mkdir l u w m
chmod 777 l u
touch l/foo
ln l/foo l/link
chmod 666 l/foo
mount -t overlay none -olowerdir=l,upperdir=u,workdir=w m su fsgqa
ln m/foo m/bar

 [   21.455823] overlayfs: failed to set redirect (-1)
 ln: failed to create hard link 'm/bar' => 'm/foo':\
     Invalid cross-device link
Fixes 4120fe64dce4 ("ovl: Set redirect on upper inode when it is linked")
Cc <[email protected]> # v4.19
---
 fs/overlayfs/dir.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
fs/overlayfs/dir.c +13 -1
index c6289147c787..82c129bfe58d 100644
@@ -651,6 +651,18 @@ static int ovl_symlink(struct inode *dir, struct dentry *dentry,
 	return ovl_create_object(dentry, S_IFLNK, 0, link);
 }
 
+static int ovl_set_link_redirect(struct dentry *dentry)
+{
+	const struct cred *old_cred;
+	int err;
+
+	old_cred = ovl_override_creds(dentry->d_sb);
+	err = ovl_set_redirect(dentry, false);
+	revert_creds(old_cred);
+
+	return err;
+}
+
 static int ovl_link(struct dentry *old, struct inode *newdir,
 		    struct dentry *new)
 {
@@ -670,7 +682,7 @@ static int ovl_link(struct dentry *old, struct inode *newdir,
 		goto out_drop_write;
 
 	if (ovl_is_metacopy_dentry(old)) {
-		err = ovl_set_redirect(old, false);
+		err = ovl_set_link_redirect(old);
 		if (err)
 			goto out_drop_write;
 	}
Signature · 1 line
2.17.1
Acked-by on v1 1/1
#1 lore raw
Quoted 9 lines · Amir Goldstein

Theodore Ts'o reported a v4.19 regression with docker-dropbox:
https://marc.info/?l=linux-fsdevel&m=154070089431116&w=2

"I was rebuilding my dropbox Docker container, and it failed in 4.19
with the following error:
...
dpkg: error: error creating new backup file \
'/var/lib/dpkg/status-old': Invalid cross-device link"

Quoted 4 lines

The problem did not reproduce with metacopy feature disabled. The error was caused by insufficient credentials to set "trusted.overlay.redirect" xattr on link of a metacopy file.

Thanks Amir for fixiing this. This fix looks good to me.

So this problem happened because looks like kernel enanbled metacopy by default (in Kconfig) and thence enabled redirect by default in Kconfig.

I am wondering how does that impact docker build process. Because if redirect/metacopy is enabled, while during diff, docker falls back to using older naivediff interface as opposed to newer nativediff. I am not sure how slower naivediff is as compared to nativediff. I am raising this point here because I am curious. If somebody has that info, please share.

Thanks
Vivek

Quoted 61 lines

Reproducer:

echo Y > /sys/module/overlay/parameters/redirect_dir echo Y > /sys/module/overlay/parameters/metacopy cd /tmp
mkdir l u w m
chmod 777 l u
touch l/foo
ln l/foo l/link
chmod 666 l/foo
mount -t overlay none -olowerdir=l,upperdir=u,workdir=w m su fsgqa
ln m/foo m/bar

 [   21.455823] overlayfs: failed to set redirect (-1)
 ln: failed to create hard link 'm/bar' => 'm/foo':\
     Invalid cross-device link
Fixes 4120fe64dce4 ("ovl: Set redirect on upper inode when it is linked")
Cc <[email protected]> # v4.19
---
 fs/overlayfs/dir.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
fs/overlayfs/dir.c +13 -1
index c6289147c787..82c129bfe58d 100644
@@ -651,6 +651,18 @@ static int ovl_symlink(struct inode *dir, struct dentry *dentry,
 	return ovl_create_object(dentry, S_IFLNK, 0, link);
 }
 
+static int ovl_set_link_redirect(struct dentry *dentry)
+{
+	const struct cred *old_cred;
+	int err;
+
+	old_cred = ovl_override_creds(dentry->d_sb);
+	err = ovl_set_redirect(dentry, false);
+	revert_creds(old_cred);
+
+	return err;
+}
+
 static int ovl_link(struct dentry *old, struct inode *newdir,
 		    struct dentry *new)
 {
@@ -670,7 +682,7 @@ static int ovl_link(struct dentry *old, struct inode *newdir,
 		goto out_drop_write;
 
 	if (ovl_is_metacopy_dentry(old)) {
-		err = ovl_set_redirect(old, false);
+		err = ovl_set_link_redirect(old);
 		if (err)
 			goto out_drop_write;
 	}
Signature · 1 line
2.17.1
maintainer
#2 lore raw
Quoted 15 lines · Vivek Goyal
Quoted 9 lines · Amir Goldstein

Theodore Ts'o reported a v4.19 regression with docker-dropbox:
https://marc.info/?l=linux-fsdevel&m=154070089431116&w=2

"I was rebuilding my dropbox Docker container, and it failed in 4.19
with the following error:
...
dpkg: error: error creating new backup file \
'/var/lib/dpkg/status-old': Invalid cross-device link"

Quoted 4 lines

The problem did not reproduce with metacopy feature disabled. The error was caused by insufficient credentials to set "trusted.overlay.redirect" xattr on link of a metacopy file.

Quoted 6 lines

Thanks Amir for fixiing this. This fix looks good to me.

So this problem happened because looks like kernel enanbled metacopy by default (in Kconfig) and thence enabled redirect by default in Kconfig.

OVERLAY_FS_METACOPY is off by default. So it seems people explicitly enable it, because it sounds useful :)

Thanks,
Miklos