[RFC][PATCH] ovl: unbless lower st_ino of files inside redirected parent dir

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On a malformed overlay, several redirected dirs can point to the same
dir on a lower layer. This presents a similar challenge as broken
hardlinks, because different objects in the overlay can return the same
st_ino/st_dev pair from stat(2).

For broken hardlinks, we do not provide constant st_ino on copy up to
avoid this inconsistency. Apply the same logic to files nested under
redirected parent dirs.

Until index for directories is implemented, every redirected dir is a
suspect of multiply referenced lower dir.

Multiple non-dir upper can also point at a single non-hardlink lower by
origin xattr causing the same challenge, but this patch doesn't address
this case.

Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
---

Miklos,

I am currently working on 'verify_dir' feature.
With 'verify_dir=on', directories are indexed on copy up and index is
checked on lookup of directories.

'verify_dir' can guaranty (*) that:
1. At most one upper dir points to the same merge dir lower
2. st_ino/st_dev of merge dir is unique
3. decode_fh(encode_fh(dir)) is idempotent
4. Stale 'origin' is permanently corrected on lookup by setting 'opaque'
5. Mismatch 'origin' is permanently corrected on lookup by setting 'redirect'

At the moment, the introduction of 'redirect_dir' feature and commit
72b608f08528 ("ovl: constant st_ino/st_dev across copy up") leave
overlayfs exposed to breaking consistency rules #2 above.

I wanted to know where stand w.r.t to the this issue:
- Not a kernel problem - should be addressed by fsck.overlayfs?
- Patch should be applied to stable v4.12 with another patch to address
  non-dir rename?
- Problem should be addressed only when opting-in to verify_dir=on,
  along with the rest of the listed consistency guaranties?

(*) At this time, I have no solution for rules #1, #2, #5 when
    numlower > 1, so I plan to emit a warning on mount with verify_dir=on
    and excersize best effort w.r.t those rules.

Thought and comments are welcome.

Thanks,
Amir.

 fs/overlayfs/inode.c     |  6 +++++-
 fs/overlayfs/namei.c     |  4 ++++
 fs/overlayfs/overlayfs.h |  4 ++++
 fs/overlayfs/util.c      | 30 ++++++++++++++++++++++++++++++
 4 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 00b6b294272a..625a31821517 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -105,11 +105,15 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
 			 * Lower hardlinks may be broken on copy up to different
 			 * upper files, so we cannot use the lower origin st_ino
 			 * for those different files, even for the same fs case.
+			 * Similarly, several redirected dirs can point to the
+			 * same dir on a lower layer, so we cannot use the lower
+			 * origin st_ino, if any parent is redirected.
 			 * With inodes index enabled, it is safe to use st_ino
 			 * of an indexed hardlinked origin. The index validates
 			 * that the upper hardlink is not broken.
 			 */
-			if (is_dir || lowerstat.nlink == 1 ||
+			if (((is_dir || lowerstat.nlink == 1) &&
+			     !ovl_dentry_is_renamed(dentry)) ||
 			    ovl_test_flag(OVL_INDEX, d_inode(dentry)))
 				stat->ino = lowerstat.ino;
 
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index b6564f1a6d48..4a5c23260473 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -724,6 +724,10 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
 			goto out_free_oe;
 
 		OVL_I(inode)->redirect = upperredirect;
+		/* OVL_RENAMED indicates rename in any layer/path component */
+		if (d.redirect ||
+		    ovl_test_flag(OVL_RENAMED, d_inode(dentry->d_parent)))
+			ovl_set_flag(OVL_RENAMED, inode);
 		if (index)
 			ovl_set_flag(OVL_INDEX, inode);
 	}
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 13eab09a6b6f..5daa1a84f217 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -33,7 +33,10 @@ enum ovl_flag {
 	OVL_IMPURE,
 	/* Non-merge dir that may contain whiteout entries */
 	OVL_WHITEOUTS,
+	/* Found index entry for origin inode */
 	OVL_INDEX,
+	/* Merge dir in path was redirected or non-dir renamed */
+	OVL_RENAMED,
 };
 
 /*
@@ -218,6 +221,7 @@ void ovl_dentry_set_upper_alias(struct dentry *dentry);
 bool ovl_redirect_dir(struct super_block *sb);
 const char *ovl_dentry_get_redirect(struct dentry *dentry);
 void ovl_dentry_set_redirect(struct dentry *dentry, const char *redirect);
+bool ovl_dentry_is_renamed(struct dentry *dentry);
 void ovl_inode_init(struct inode *inode, struct dentry *upperdentry,
 		    struct dentry *lowerdentry);
 void ovl_inode_update(struct inode *inode, struct dentry *upperdentry);
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index d6bb1c9f5e7a..2876492a86e5 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -249,6 +249,36 @@ void ovl_dentry_set_redirect(struct dentry *dentry, const char *redirect)
 
 	kfree(oi->redirect);
 	oi->redirect = redirect;
+	/* Descentandt are marekd RENAMED lazily by ovl_dentry_is_renamed() */
+	ovl_set_flag(OVL_RENAMED, d_inode(dentry));
+}
+
+/*
+ * Check if any component in path has been renamed.
+ *
+ * TODO: lookup non-indexed non-dir upper with non-hardlink origin in lower
+ *       layers and mark it RENAMED if origin not found.
+ */
+bool ovl_dentry_is_renamed(struct dentry *dentry)
+{
+	struct dentry *d, *tmp;
+	bool renamed;
+
+	for (d = dget(dentry); !IS_ROOT(d);) {
+		if (ovl_test_flag(OVL_RENAMED, d_inode(d)))
+			break;
+
+		tmp = dget_parent(d);
+		dput(d);
+		d = tmp;
+	}
+
+	renamed = !IS_ROOT(d);
+	dput(d);
+
+	if (renamed)
+		ovl_set_flag(OVL_RENAMED, d_inode(dentry));
+	return renamed;
 }
 
 void ovl_inode_init(struct inode *inode, struct dentry *upperdentry,
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux