Re: [PATCH 4/4] vfs: fs/namei.c: if RESOLVE_NO_XDEV passed to openat2, don't *trigger* automounts

On 2025-08-18, Aleksa Sarai <[email protected]> wrote:
> On 2025-08-17, Askar Safin <[email protected]> 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/[email protected]/
> > Fixes: fddb5d430ad9fa91b49b1 ("open: introduce openat2(2) syscall")
> > Signed-off-by: Askar Safin <[email protected]>
> 
> Reviewed-by: Aleksa Sarai <[email protected]>
> 
> > ---
> >  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/

-----BEGIN PGP SIGNATURE-----

iJEEABYKADkWIQS2TklVsp+j1GPyqQYol/rSt+lEbwUCaKLTBBsUgAAAAAAEAA5t
YW51MiwyLjUrMS4xMSwyLDIACgkQKJf60rfpRG+yDQEA9cFyoq35bvrj9ODvEWTd
9GwkZfz/O6WfZW6/sIGkGxMBAPv3r23XydNZUkFXK4URwlrlW+jiLrCCvhvkjgc3
KK4M
=Wh/y
-----END PGP SIGNATURE-----