| Message ID | 1430803373-4948-47-git-send-email-viro@ZenIV.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 5 May 2015 06:22:21 +0100 Al Viro <viro@ZenIV.linux.org.uk> wrote: > From: Al Viro <viro@zeniv.linux.org.uk> > > Array of MAX_NESTED_LINKS + 1 elements put into nameidata; > what used to be a local array in link_path_walk() occupies > entries 1 .. MAX_NESTED_LINKS in it, link and cookie from > the trailing symlink handling loops - entry 0. > > This is _not_ the final arrangement; just an easily verified > incremental step. > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- > fs/namei.c | 35 ++++++++++++++++++----------------- > 1 file changed, 18 insertions(+), 17 deletions(-) > > diff --git a/fs/namei.c b/fs/namei.c > index 38ff968..e6d54ec 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -505,6 +505,11 @@ struct nameidata { > int last_type; > unsigned depth; > struct file *base; > + struct saved { > + struct path link; > + void *cookie; > + const char *name; > + } stack[MAX_NESTED_LINKS + 1]; > }; > > /* > @@ -1703,11 +1708,7 @@ static inline u64 hash_name(const char *name) > */ > static int link_path_walk(const char *name, struct nameidata *nd) > { > - struct saved { > - struct path link; > - void *cookie; > - const char *name; > - } stack[MAX_NESTED_LINKS], *last = stack + nd->depth - 1; > + struct saved *last = nd->stack; This looks wrong. 'last' used to be dependent on nd->depth, now it isn't. Three patches later we have: +const char *get_link(struct nameidata *nd) { - struct dentry *dentry = link->dentry; + struct saved *last = nd->stack + nd->depth; + struct dentry *dentry = nd->link.dentry; struct inode *inode = dentry->d_inode; and static int link_path_walk(const char *name, struct nameidata *nd) { - struct saved *last = nd->stack; int err; .... - last->link = nd->link; - s = get_link(&last->link, nd, &last->cookie); + s = get_link(nd); so the nd->depth offset is restored in get_link() so the next result appears correct, but the intermediate states appear to be wrong. NeilBrown
On Wed, May 06, 2015 at 12:42:54PM +1000, NeilBrown wrote: > > static int link_path_walk(const char *name, struct nameidata *nd) > > { > > - struct saved { > > - struct path link; > > - void *cookie; > > - const char *name; > > - } stack[MAX_NESTED_LINKS], *last = stack + nd->depth - 1; > > + struct saved *last = nd->stack; > > This looks wrong. 'last' used to be dependent on nd->depth, now it isn't. On the entry to link_path_walk() we have nd->depth == 0. And nd->depth and last are changed in sync. -- 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 38ff968..e6d54ec 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -505,6 +505,11 @@ struct nameidata { int last_type; unsigned depth; struct file *base; + struct saved { + struct path link; + void *cookie; + const char *name; + } stack[MAX_NESTED_LINKS + 1]; }; /* @@ -1703,11 +1708,7 @@ static inline u64 hash_name(const char *name) */ static int link_path_walk(const char *name, struct nameidata *nd) { - struct saved { - struct path link; - void *cookie; - const char *name; - } stack[MAX_NESTED_LINKS], *last = stack + nd->depth - 1; + struct saved *last = nd->stack; int err; while (*name=='/') @@ -2022,13 +2023,13 @@ static int path_lookupat(int dfd, const struct filename *name, if (!err && !(flags & LOOKUP_PARENT)) { err = lookup_last(nd); while (err > 0) { - void *cookie; - struct path link = nd->link; - err = trailing_symlink(&link, nd, &cookie); + nd->stack[0].link = nd->link; + err = trailing_symlink(&nd->stack[0].link, + nd, &nd->stack[0].cookie); if (err) break; err = lookup_last(nd); - put_link(nd, &link, cookie); + put_link(nd, &nd->stack[0].link, nd->stack[0].cookie); } } @@ -2368,13 +2369,13 @@ path_mountpoint(int dfd, const struct filename *name, struct path *path, err = mountpoint_last(nd, path); while (err > 0) { - void *cookie; - struct path link = *path; - err = trailing_symlink(&link, nd, &cookie); + nd->stack[0].link = nd->link; + err = trailing_symlink(&nd->stack[0].link, + nd, &nd->stack[0].cookie); if (err) break; err = mountpoint_last(nd, path); - put_link(nd, &link, cookie); + put_link(nd, &nd->stack[0].link, nd->stack[0].cookie); } out: path_cleanup(nd); @@ -3253,14 +3254,14 @@ static struct file *path_openat(int dfd, struct filename *pathname, error = do_last(nd, file, op, &opened, pathname); while (unlikely(error > 0)) { /* trailing symlink */ - struct path link = nd->link; - void *cookie; nd->flags &= ~(LOOKUP_OPEN|LOOKUP_CREATE|LOOKUP_EXCL); - error = trailing_symlink(&link, nd, &cookie); + nd->stack[0].link = nd->link; + error= trailing_symlink(&nd->stack[0].link, + nd, &nd->stack[0].cookie); if (unlikely(error)) break; error = do_last(nd, file, op, &opened, pathname); - put_link(nd, &link, cookie); + put_link(nd, &nd->stack[0].link, nd->stack[0].cookie); } out: path_cleanup(nd);