On 2025-08-18, Aleksa Sarai <cyphar@xxxxxxxxxx> wrote: > On 2025-08-17, Askar Safin <safinaskar@xxxxxxxxxxxx> wrote: > > openat2 had a bug: if we pass RESOLVE_NO_XDEV, then openat2 > > doesn't traverse through automounts, but may still trigger them. > > (See the link for full bug report with reproducer.) > > > > This commit fixes this bug. > > > > Link: https://lore.kernel.org/linux-fsdevel/20250817075252.4137628-1-safinaskar@xxxxxxxxxxxx/ > > Fixes: fddb5d430ad9fa91b49b1 ("open: introduce openat2(2) syscall") > > Signed-off-by: Askar Safin <safinaskar@xxxxxxxxxxxx> > > Reviewed-by: Aleksa Sarai <cyphar@xxxxxxxxxx> > > > --- > > fs/namei.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/fs/namei.c b/fs/namei.c > > index 6f43f96f506d..55e2447699a4 100644 > > --- a/fs/namei.c > > +++ b/fs/namei.c > > @@ -1449,6 +1449,18 @@ static int follow_automount(struct path *path, int *count, unsigned lookup_flags > > dentry->d_inode) > > return -EISDIR; > > > > + /* "if" above returned -EISDIR if we want to get automount point itself > > + * as opposed to new mount. Getting automount point itself is, of course, > > + * totally okay even if we have LOOKUP_NO_XDEV. > > + * > > + * But if we got here, then we want to get > > + * new mount. Let's deny this if LOOKUP_NO_XDEV is specified. > > + * If we have LOOKUP_NO_XDEV, then we want to deny not only > > + * traversing through automounts, but also triggering them > > + */ > > This comment really could be one sentence: > > /* No need to trigger automounts if mountpoint crossing is disabled. */ > > Or if you really want to mention -EISDIR, then: > > /* > * No need to trigger automounts if mountpoint crossing is disabled. > * If the caller is trying to check the autmount point itself, -EISDIR > * above handles that case for us. > */ That being said, the only user of LOOKUP_NO_XDEV is openat2(), so the stat stuff is a bit of a red herring. At the moment, O_PATH and O_PATH|O_DIRECTORY have different behaviours here, and I'm not sure users expect that -- O_PATH will let you get a handle to the automount point, but O_DIRECTORY causes the -EISDIR case to be skipped and triggers the automount. We can't just remove LOOKUP_DIRECTORY from the check since it is used for a lot of kernel-internal lookups, but we should have O_PATH|O_DIRECTORY produce identical behaviour to O_PATH in this case IMHO. > > + if (lookup_flags & LOOKUP_NO_XDEV) > > + return -EXDEV; > > + > > if (count && (*count)++ >= MAXSYMLINKS) > > return -ELOOP; > > > > @@ -1472,6 +1484,10 @@ static int __traverse_mounts(struct path *path, unsigned flags, bool *jumped, > > /* Allow the filesystem to manage the transit without i_rwsem > > * being held. */ > > if (flags & DCACHE_MANAGE_TRANSIT) { > > + if (lookup_flags & LOOKUP_NO_XDEV) { > > + ret = -EXDEV; > > + break; > > + } > > ret = path->dentry->d_op->d_manage(path, false); > > flags = smp_load_acquire(&path->dentry->d_flags); > > if (ret < 0) > > -- > > 2.47.2 > > > > -- > Aleksa Sarai > Senior Software Engineer (Containers) > SUSE Linux GmbH > https://www.cyphar.com/ -- Aleksa Sarai Senior Software Engineer (Containers) SUSE Linux GmbH https://www.cyphar.com/
Attachment:
signature.asc
Description: PGP signature