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
