On Tue, Jan 9, 2018 at 3:31 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > On Thu, Jan 4, 2018 at 5:40 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: >> 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. When "verify" feature is enabled, apply >> the same logic to files nested under unverified redirected dirs. >> >> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> >> --- >> fs/overlayfs/inode.c | 18 ++++++++++++++---- >> fs/overlayfs/namei.c | 4 ++++ >> fs/overlayfs/overlayfs.h | 4 ++++ >> fs/overlayfs/util.c | 27 +++++++++++++++++++++++++++ >> 4 files changed, 49 insertions(+), 4 deletions(-) >> >> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c >> index 00b6b294272a..1b53755bd86c 100644 >> --- a/fs/overlayfs/inode.c >> +++ b/fs/overlayfs/inode.c >> @@ -105,12 +105,22 @@ 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. With the "verify" feature, >> + * we do not use the lower origin st_ino, if any parent >> + * is redirected and we haven't verified that this >> + * redirect is unique. >> + * >> * 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. >> + * of an indexed origin. The index validates that the >> + * upper hardlink is not broken and that a redirected >> + * dir is the only redirect to that origin. >> */ >> - if (is_dir || lowerstat.nlink == 1 || >> - ovl_test_flag(OVL_INDEX, d_inode(dentry))) >> + if (ovl_test_flag(OVL_INDEX, d_inode(dentry)) || >> + ((is_dir || lowerstat.nlink == 1) && >> + (!ovl_verify(dentry->d_sb) || >> + !ovl_dentry_is_renamed(dentry)))) > > Won't this make st_ino change when an ancestor directory is renamed? > Even if there is no "double redirect" that we are fearing? If so, > than I think the cure is worse than the disease. > Right. I should drop ovl_dentry_is_renamed(). It was ill conceived. For verify=off, we trust lower st_ino for non-indexed dir and non-hardlink. For verify=on, we only trust lower st_ino for indexed dir and non-dir. Using the upper st_ino for non-indexed upper with verify=on is consistent with the fact that we also encode an upper file handle for non-indexed upper. That means that st_ino remains constant on copy up for both verify=on/off, but when same overlay is mounted verify=off and then verify=on, st_ino of an already copied up (and non-indexed) file st_ino will change. I don't think that behavior is a problem, considering the fact that merge dir without verified origin will also not behave the same on verify=on/off cases. Thanks, Amir. -- 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
