| Message ID | 20160702005525.GA28451@vader.dhcp.thefacebook.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jul 1, 2016 at 5:55 PM, Omar Sandoval <osandov@osandov.com> wrote: > On Tue, Jun 28, 2016 at 10:38:28AM -0700, Andrey Vagin wrote: >> The problem is that a pathname can contain absolute symlinks and now >> they are resolved relative to the current root. >> >> If we want to open a file in another mount namespaces and we have a file >> descriptor to its root directory, we probably want to resolve pathname >> in the target mount namespace. For this we add this new flag. >> >> If LOOKUP_DFD_ROOT is set, path_init() initializes nd->root and nd->path >> to the same value. >> >> Signed-off-by: Andrey Vagin <avagin@openvz.org> > > Hi, Andrey, > > Seems like a useful feature. Make sure to cc linux-api@vger.kernel.org > for new userspace interfaces. One comment on the implementation below. Hi Omar, Thank you for the patch. I have sent a new version with your changes. If there will not be other comments in a few days, I will resent a whole seires with linux-api@vger.kernel.org in CC. Thanks, Andrew > >> --- >> fs/namei.c | 12 +++++++++++- >> include/linux/namei.h | 2 ++ >> 2 files changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/fs/namei.c b/fs/namei.c >> index 70580ab..5f08b69 100644 >> --- a/fs/namei.c >> +++ b/fs/namei.c >> @@ -2148,7 +2148,7 @@ static const char *path_init(struct nameidata *nd, unsigned flags) >> nd->path.dentry = NULL; >> >> nd->m_seq = read_seqbegin(&mount_lock); >> - if (*s == '/') { >> + if (*s == '/' && !(flags & LOOKUP_DFD_ROOT)) { >> if (flags & LOOKUP_RCU) >> rcu_read_lock(); >> set_root(nd); >> @@ -2174,6 +2174,11 @@ static const char *path_init(struct nameidata *nd, unsigned flags) >> get_fs_pwd(current->fs, &nd->path); >> nd->inode = nd->path.dentry->d_inode; >> } >> + if (flags & LOOKUP_DFD_ROOT) { >> + nd->root = nd->path; >> + if (!(flags & LOOKUP_RCU)) >> + path_get(&nd->root); > > You're not initializing nd->root_seq here. That means that if we end up > going through unlazy_walk(), we're going to call legitimize_path() (and > thus read_seqcount_retry()) with stack garbage, get a spurious ECHILD, > and do an unnecessary restart of the path lookup instead of dropping > into ref-walk mode. > >> + } >> return s; >> } else { >> /* Caller must check execute permissions on the starting path component */ >> @@ -2202,6 +2207,11 @@ static const char *path_init(struct nameidata *nd, unsigned flags) >> nd->inode = nd->path.dentry->d_inode; >> } >> fdput(f); >> + if (flags & LOOKUP_DFD_ROOT) { >> + nd->root = nd->path; >> + if (!(flags & LOOKUP_RCU)) >> + path_get(&nd->root); >> + } > > Same here. > > The following should do the trick: > > diff --git a/fs/namei.c b/fs/namei.c > index 9958b605e822..101d1fb8d3cb 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -2176,7 +2176,9 @@ static const char *path_init(struct nameidata *nd, unsigned flags) > } > if (flags & LOOKUP_DFD_ROOT) { > nd->root = nd->path; > - if (!(flags & LOOKUP_RCU)) > + if (flags & LOOKUP_RCU) > + nd->root_seq = nd->seq; > + else > path_get(&nd->root); > } > return s; > @@ -2209,7 +2211,9 @@ static const char *path_init(struct nameidata *nd, unsigned flags) > fdput(f); > if (flags & LOOKUP_DFD_ROOT) { > nd->root = nd->path; > - if (!(flags & LOOKUP_RCU)) > + if (flags & LOOKUP_RCU) > + nd->root_seq = nd->seq; > + else > path_get(&nd->root); > } > return s; > > -- > Omar > _______________________________________________ > Containers mailing list > Containers@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/containers -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/namei.c b/fs/namei.c index 9958b605e822..101d1fb8d3cb 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2176,7 +2176,9 @@ static const char *path_init(struct nameidata *nd, unsigned flags) } if (flags & LOOKUP_DFD_ROOT) { nd->root = nd->path; - if (!(flags & LOOKUP_RCU)) + if (flags & LOOKUP_RCU) + nd->root_seq = nd->seq; + else path_get(&nd->root); } return s; @@ -2209,7 +2211,9 @@ static const char *path_init(struct nameidata *nd, unsigned flags) fdput(f); if (flags & LOOKUP_DFD_ROOT) { nd->root = nd->path; - if (!(flags & LOOKUP_RCU)) + if (flags & LOOKUP_RCU) + nd->root_seq = nd->seq; + else path_get(&nd->root); } return s;