On 2019-11-13, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
On Wed, Nov 13, 2019 at 01:44:14PM +1100, Aleksa Sarai wrote:On 2019-11-13, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:On Tue, Nov 05, 2019 at 08:05:49PM +1100, Aleksa Sarai wrote:@@ -2277,12 +2277,20 @@ static const char *path_init(struct nameidata *nd, unsigned flags) nd->m_seq = read_seqbegin(&mount_lock); - /* Figure out the starting path and root (if needed). */ - if (*s == '/') { + /* Absolute pathname -- fetch the root. */ + if (flags & LOOKUP_IN_ROOT) { + /* With LOOKUP_IN_ROOT, act as a relative path. */ + while (*s == '/') + s++;Er... Why bother skipping slashes? I mean, not only link_path_walk() will skip them just fine, you are actually risking breakage in this: if (*s && unlikely(!d_can_lookup(dentry))) { fdput(f); return ERR_PTR(-ENOTDIR); } which is downstream from there with you patch, AFAICS.I switched to stripping the slashes at your suggestion a few revisions ago[1], and had (wrongly) assumed we needed to handle "/" somehow in path_init(). But you're quite right about link_path_walk() -- and I'd be more than happy to drop it.That, IIRC, was about untangling the weirdness around multiple calls of dirfd_path_init() and basically went "we might want just strip the slashes in case of that flag very early in the entire thing, so that later the normal logics for absolute/relative would DTRT".
Ah okay, I'd misunderstood the point you were making in that thread.
Since your check is right next to checking for absolute pathnames (and not in the very beginning of path_init()), we might as well turn the check for absolute pathname into *s == '/' && !(flags & LOOKUP_IN_ROOT) and be done with that.
Yup, agreed. -- Aleksa Sarai Senior Software Engineer (Containers) SUSE Linux GmbH <https://www.cyphar.com/>
Attachment:
signature.asc
Description: PGP signature
