On Wed, Oct 9, 2019 at 10:42 PM Aleksa Sarai cyphar@cyphar.com wrote:
--- a/fs/namei.c +++ b/fs/namei.c @@ -2277,6 +2277,11 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
nd->m_seq = read_seqbegin(&mount_lock);
/* LOOKUP_IN_ROOT treats absolute paths as being relative-to-dirfd. */if (flags & LOOKUP_IN_ROOT)while (*s == '/')s++;/* Figure out the starting path and root (if needed). */ if (*s == '/') { error = nd_jump_root(nd);
Hmm. Wouldn't this make more sense all inside the if (*s =- '/') test? That way if would be where we check for "should we start at the root", which seems to make more sense conceptually.
That test for '/' currently has a "} else if (..)", but that's pointless since it ends with a "return" anyway. So the "else" logic is just noise.
And if you get rid of the unnecessary else, moving the LOOKUP_IN_ROOT inside the if-statement works fine.
So this could be something like
--- a/fs/namei.c +++ b/fs/namei.c @@ -2194,11 +2196,19 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
nd->m_seq = read_seqbegin(&mount_lock); if (*s == '/') { - set_root(nd); - if (likely(!nd_jump_root(nd))) - return s; - return ERR_PTR(-ECHILD); - } else if (nd->dfd == AT_FDCWD) { + /* LOOKUP_IN_ROOT treats absolute paths as being relative-to-dirfd. */ + if (!(flags & LOOKUP_IN_ROOT)) { + set_root(nd); + if (likely(!nd_jump_root(nd))) + return s; + return ERR_PTR(-ECHILD); + } + + /* Skip initial '/' for LOOKUP_IN_ROOT */ + do { s++; } while (*s == '/'); + } + + if (nd->dfd == AT_FDCWD) { if (flags & LOOKUP_RCU) { struct fs_struct *fs = current->fs; unsigned seq;
instead. The patch ends up slightly bigger (due to the re-indentation) but now it handles all the "start at root" in the same place. Doesn't that make sense?
Linus