Serious error in autofs docs, which has design implications

Serious error in autofs docs, which has design implications

[Askar Safin]

Hi, Ian Kent and other autofs people.

autofs.rst says:
> mounting onto a directory is considered to be "beyond a `stat`"
in https://elixir.bootlin.com/linux/v6.17-rc2/source/Documentation/filesystems/autofs.rst#L109

This is not true. Mounting does not trigger automounts.

mount syscall (
https://elixir.bootlin.com/linux/v6.17-rc2/source/fs/namespace.c#L4321
) calls "do_mount" (
https://elixir.bootlin.com/linux/v6.17-rc2/source/fs/namespace.c#L4124
), which calls "user_path_at" without LOOKUP_AUTOMOUNT.
This means automounts are not followed.
I didn't test this, but I'm pretty sure about this by reading code.

But what is worse, autofs.rst then proceeds to use this as an argument in
favor of introducing DCACHE_MANAGE_TRANSIT!

I. e. it seems that introducing DCACHE_MANAGE_TRANSIT rests on
wrong premise.

Thus, it seems (from reading autofs.rst) that DCACHE_MANAGE_TRANSIT and all accociated logic
can be removed from kernel.

--
Askar Safin
https://types.pl/@safinaskar

Re: Serious error in autofs docs, which has design implications

[Ian Kent]

On 21/8/25 15:53, Askar Safin wrote:
> Hi, Ian Kent and other autofs people.
>
> autofs.rst says:
>> mounting onto a directory is considered to be "beyond a `stat`"
> in https://elixir.bootlin.com/linux/v6.17-rc2/source/Documentation/filesystems/autofs.rst#L109
>
> This is not true. Mounting does not trigger automounts.

I don't understand that statement either, it's been many years

since this was done and I can't remember the exact details. I

didn't write this Neil Brown did and I have spoken with Neil

many times over the years and although I'm quite sure we talked

about this document at the time it was so long I don't actually

remember what was said.


It's quite likely that what I said at the time was misunderstood

but the discussion following this is reasonably clear and describes

the uses of the callback. The need to not race with an ongoing

expire of an automounted mount is important and the transit through

a non-empty directory in the case the automount map in use specifies

a tree of mounts is important too (no this is not kernel only

automounting such as is used by the likes of NFS it's functionality

implemented by automount(8) based the mount map constructs it uses).

So much of this is needed by the autofs file system during path

traversal which is used by automount (not general file systems).


>
> mount syscall (
> https://elixir.bootlin.com/linux/v6.17-rc2/source/fs/namespace.c#L4321
> ) calls "do_mount" (
> https://elixir.bootlin.com/linux/v6.17-rc2/source/fs/namespace.c#L4124
> ), which calls "user_path_at" without LOOKUP_AUTOMOUNT.
> This means automounts are not followed.
> I didn't test this, but I'm pretty sure about this by reading code.

Explain what you mean please.


>
> But what is worse, autofs.rst then proceeds to use this as an argument in
> favor of introducing DCACHE_MANAGE_TRANSIT!

I don't think that's the way you should be looking at this.


At the time this was implemented into the VFS there were several things

that autofs needed to do for automount(8) and David Howells chose to do

it this way at the time after discussing what was needed with me.


>
> I. e. it seems that introducing DCACHE_MANAGE_TRANSIT rests on
> wrong premise.

I don't think so myself.


But it may be possible to do it differently if there are reasons to

do so.


IIRC Al doesn't much like this either but even so I would need a clear

description and discussion of how the cases I need are covered before

changing this to some other method.


>
> Thus, it seems (from reading autofs.rst) that DCACHE_MANAGE_TRANSIT and all accociated logic
> can be removed from kernel.

Again I don't think that's the case at all, certainly automount(8) will

see various breakage without changes to it and probably the autofs file

system.


IIRC (and I likely don't) I would probably need to re-introduce

->d_revalidate() to autofs and make sure that the VFS locking is

consistent (which it wasn't at the when this was originally done

and what was needed didn't seem to fit sensibly into ->d_revalidate()

either) wrt. autofs's needs.


So why do you need to change this?


Ian

Re: Serious error in autofs docs, which has design implications

[Askar Safin]

 ---- On Fri, 22 Aug 2025 16:31:46 +0400  Ian Kent <raven@themaw.net> wrote --- 
 > On 21/8/25 15:53, Askar Safin wrote:
 > > autofs.rst says:
 > >> mounting onto a directory is considered to be "beyond a `stat`"
 > > in https://elixir.bootlin.com/linux/v6.17-rc2/source/Documentation/filesystems/autofs.rst#L109
 > >
 > > This is not true. Mounting does not trigger automounts.
 > 
 > I don't understand that statement either, it's been many years

Let me explain.

Some syscalls follow (and trigger) automounts in last
component of path, and some - not.

stat(2) is one of syscalls, which don't follow
automounts in last component of supplied path.

Many other syscalls do follow automounts.

autofs.rst calls syscalls, which follow automounts,
as "beyond a stat".

Notably mount(2) doesn't follow automounts in second argument
(i. e. mountpoint). I know this, because I closely did read the code.
Also I did experiment (see source in the end of this letter).
Experiment was on 6.17-rc1.

But "autofs.rst" says:
> mounting onto a directory is considered to be "beyond a `stat`"

I. e. "autofs.rst" says that mount(2) does follow automounts.

This is wrong, as I explained above. (Again: I did experiment,
so I'm totally sure that this "autofs.rst" sentence is wrong.)

Moreover, then "autofs.rst" proceeds to explain why
DCACHE_MANAGE_TRANSIT was introduced, based on this wrong fact.

So it is possible that DCACHE_MANAGE_TRANSIT is in fact, not needed.

I'm not asking for removal of DCACHE_MANAGE_TRANSIT.

I merely point error in autofs.rst file and ask for fix.

And if in process of fixing autofs.rst you will notice that
DCACHE_MANAGE_TRANSIT is indeed not needed, then,
of course, it should be removed.

--
Askar Safin
https://types.pl/@safinaskar

====

// This code is public domain
// You should be root in initial user namespace

#define _GNU_SOURCE

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <sched.h>
#include <errno.h>
#include <sys/stat.h>
#include <sys/mount.h>
#include <sys/syscall.h>
#include <sys/vfs.h>
#include <sys/sysmacros.h>
#include <sys/statvfs.h>
#include <sys/wait.h>
#include <linux/openat2.h>
#include <linux/nsfs.h>

#define MY_ASSERT(cond) do { \
    if (!(cond)) { \
        fprintf (stderr, "%d: %s: assertion failed\n", __LINE__, #cond); \
        exit (1); \
    } \
} while (0)

#define MY_ASSERT_ERRNO(cond) do { \
    if (!(cond)) { \
        fprintf (stderr, "%d: %s: %m\n", __LINE__, #cond); \
        exit (1); \
    } \
} while (0)

static void
mount_debugfs (void)
{
    if (mount (NULL, "/tmp/debugfs", "debugfs", 0, NULL) != 0)
        {
            perror ("mount debugfs");
            exit (1);
        }
}

int
main (void)
{
    MY_ASSERT_ERRNO (chdir ("/") == 0);
    MY_ASSERT_ERRNO (unshare (CLONE_NEWNS) == 0);
    MY_ASSERT_ERRNO (mount (NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL) == 0);
    MY_ASSERT_ERRNO (mount (NULL, "/tmp", "tmpfs", 0, NULL) == 0);
    MY_ASSERT_ERRNO (mkdir ("/tmp/debugfs", 0777) == 0);
    mount_debugfs ();
    MY_ASSERT_ERRNO (mount (NULL, "/tmp/debugfs/tracing", "tmpfs", 0, NULL) == 0);
    execlp ("/bin/busybox", "sh", NULL);
    MY_ASSERT (false);
}

Re: Serious error in autofs docs, which has design implications

[Ian Kent]

On 25/8/25 22:38, Askar Safin wrote:
>   ---- On Fri, 22 Aug 2025 16:31:46 +0400  Ian Kent <raven@themaw.net> wrote ---
>   > On 21/8/25 15:53, Askar Safin wrote:
>   > > autofs.rst says:
>   > >> mounting onto a directory is considered to be "beyond a `stat`"
>   > > in https://elixir.bootlin.com/linux/v6.17-rc2/source/Documentation/filesystems/autofs.rst#L109
>   > >
>   > > This is not true. Mounting does not trigger automounts.
>   >
>   > I don't understand that statement either, it's been many years
>
> Let me explain.

I do understand what your saying but without more information about

the meaning and intent of the text your concerned about I don't think

anything can be done about this right now.


I guess that I should also apologise to you as I'm pretty sure I

reviewed this at the time it was posted and didn't question this

at the time. But I most likely didn't see this as a problem because,

to my thinking, what follows explains what it's needed for rather

than the earlier statement justifying it.


To be clear, in my previous reply I said two things, first I also

find the statement you are concerned about unclear, perhaps even

misleading (but I would need to understand the statement original

intent to interpret that, which I don't) and second, the ->d_namage()

callback is most definitely needed for the function of the user space

daemon, automount(8) (via the autofs file system).


Ian

>
> Some syscalls follow (and trigger) automounts in last
> component of path, and some - not.
>
> stat(2) is one of syscalls, which don't follow
> automounts in last component of supplied path.
>
> Many other syscalls do follow automounts.
>
> autofs.rst calls syscalls, which follow automounts,
> as "beyond a stat".
>
> Notably mount(2) doesn't follow automounts in second argument
> (i. e. mountpoint). I know this, because I closely did read the code.
> Also I did experiment (see source in the end of this letter).
> Experiment was on 6.17-rc1.
>
> But "autofs.rst" says:
>> mounting onto a directory is considered to be "beyond a `stat`"
> I. e. "autofs.rst" says that mount(2) does follow automounts.
>
> This is wrong, as I explained above. (Again: I did experiment,
> so I'm totally sure that this "autofs.rst" sentence is wrong.)
>
> Moreover, then "autofs.rst" proceeds to explain why
> DCACHE_MANAGE_TRANSIT was introduced, based on this wrong fact.
>
> So it is possible that DCACHE_MANAGE_TRANSIT is in fact, not needed.
>
> I'm not asking for removal of DCACHE_MANAGE_TRANSIT.
>
> I merely point error in autofs.rst file and ask for fix.
>
> And if in process of fixing autofs.rst you will notice that
> DCACHE_MANAGE_TRANSIT is indeed not needed, then,
> of course, it should be removed.
>
> --
> Askar Safin
> https://types.pl/@safinaskar
>
> ====
>
> // This code is public domain
> // You should be root in initial user namespace
>
> #define _GNU_SOURCE
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <stdbool.h>
> #include <string.h>
> #include <unistd.h>
> #include <fcntl.h>
> #include <sched.h>
> #include <errno.h>
> #include <sys/stat.h>
> #include <sys/mount.h>
> #include <sys/syscall.h>
> #include <sys/vfs.h>
> #include <sys/sysmacros.h>
> #include <sys/statvfs.h>
> #include <sys/wait.h>
> #include <linux/openat2.h>
> #include <linux/nsfs.h>
>
> #define MY_ASSERT(cond) do { \
>      if (!(cond)) { \
>          fprintf (stderr, "%d: %s: assertion failed\n", __LINE__, #cond); \
>          exit (1); \
>      } \
> } while (0)
>
> #define MY_ASSERT_ERRNO(cond) do { \
>      if (!(cond)) { \
>          fprintf (stderr, "%d: %s: %m\n", __LINE__, #cond); \
>          exit (1); \
>      } \
> } while (0)
>
> static void
> mount_debugfs (void)
> {
>      if (mount (NULL, "/tmp/debugfs", "debugfs", 0, NULL) != 0)
>          {
>              perror ("mount debugfs");
>              exit (1);
>          }
> }
>
> int
> main (void)
> {
>      MY_ASSERT_ERRNO (chdir ("/") == 0);
>      MY_ASSERT_ERRNO (unshare (CLONE_NEWNS) == 0);
>      MY_ASSERT_ERRNO (mount (NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL) == 0);
>      MY_ASSERT_ERRNO (mount (NULL, "/tmp", "tmpfs", 0, NULL) == 0);
>      MY_ASSERT_ERRNO (mkdir ("/tmp/debugfs", 0777) == 0);
>      mount_debugfs ();
>      MY_ASSERT_ERRNO (mount (NULL, "/tmp/debugfs/tracing", "tmpfs", 0, NULL) == 0);
>      execlp ("/bin/busybox", "sh", NULL);
>      MY_ASSERT (false);
> }
>

Re: Serious error in autofs docs, which has design implications

[NeilBrown]

On Tue, 26 Aug 2025, Ian Kent wrote:
> On 25/8/25 22:38, Askar Safin wrote:
> >   ---- On Fri, 22 Aug 2025 16:31:46 +0400  Ian Kent <raven@themaw.net> wrote ---
> >   > On 21/8/25 15:53, Askar Safin wrote:
> >   > > autofs.rst says:
> >   > >> mounting onto a directory is considered to be "beyond a `stat`"
> >   > > in https://elixir.bootlin.com/linux/v6.17-rc2/source/Documentation/filesystems/autofs.rst#L109
> >   > >
> >   > > This is not true. Mounting does not trigger automounts.
> >   >
> >   > I don't understand that statement either, it's been many years
> >
> > Let me explain.
> 
> I do understand what your saying but without more information about
> 
> the meaning and intent of the text your concerned about I don't think
> 
> anything can be done about this right now.

Yes, there is definitely something wrong there.  I'll try to work out
what I really meant to say and propose some alternate wording -
hopefully within a week or so.

Thanks for raising this Askar,

NeilBrown


> 
> 
> I guess that I should also apologise to you as I'm pretty sure I
> 
> reviewed this at the time it was posted and didn't question this
> 
> at the time. But I most likely didn't see this as a problem because,
> 
> to my thinking, what follows explains what it's needed for rather
> 
> than the earlier statement justifying it.
> 
> 
> To be clear, in my previous reply I said two things, first I also
> 
> find the statement you are concerned about unclear, perhaps even
> 
> misleading (but I would need to understand the statement original
> 
> intent to interpret that, which I don't) and second, the ->d_namage()
> 
> callback is most definitely needed for the function of the user space
> 
> daemon, automount(8) (via the autofs file system).
> 
> 
> Ian
> 
> >
> > Some syscalls follow (and trigger) automounts in last
> > component of path, and some - not.
> >
> > stat(2) is one of syscalls, which don't follow
> > automounts in last component of supplied path.
> >
> > Many other syscalls do follow automounts.
> >
> > autofs.rst calls syscalls, which follow automounts,
> > as "beyond a stat".
> >
> > Notably mount(2) doesn't follow automounts in second argument
> > (i. e. mountpoint). I know this, because I closely did read the code.
> > Also I did experiment (see source in the end of this letter).
> > Experiment was on 6.17-rc1.
> >
> > But "autofs.rst" says:
> >> mounting onto a directory is considered to be "beyond a `stat`"
> > I. e. "autofs.rst" says that mount(2) does follow automounts.
> >
> > This is wrong, as I explained above. (Again: I did experiment,
> > so I'm totally sure that this "autofs.rst" sentence is wrong.)
> >
> > Moreover, then "autofs.rst" proceeds to explain why
> > DCACHE_MANAGE_TRANSIT was introduced, based on this wrong fact.
> >
> > So it is possible that DCACHE_MANAGE_TRANSIT is in fact, not needed.
> >
> > I'm not asking for removal of DCACHE_MANAGE_TRANSIT.
> >
> > I merely point error in autofs.rst file and ask for fix.
> >
> > And if in process of fixing autofs.rst you will notice that
> > DCACHE_MANAGE_TRANSIT is indeed not needed, then,
> > of course, it should be removed.
> >
> > --
> > Askar Safin
> > https://types.pl/@safinaskar
> >
> > ====
> >
> > // This code is public domain
> > // You should be root in initial user namespace
> >
> > #define _GNU_SOURCE
> >
> > #include <stdio.h>
> > #include <stdlib.h>
> > #include <stdbool.h>
> > #include <string.h>
> > #include <unistd.h>
> > #include <fcntl.h>
> > #include <sched.h>
> > #include <errno.h>
> > #include <sys/stat.h>
> > #include <sys/mount.h>
> > #include <sys/syscall.h>
> > #include <sys/vfs.h>
> > #include <sys/sysmacros.h>
> > #include <sys/statvfs.h>
> > #include <sys/wait.h>
> > #include <linux/openat2.h>
> > #include <linux/nsfs.h>
> >
> > #define MY_ASSERT(cond) do { \
> >      if (!(cond)) { \
> >          fprintf (stderr, "%d: %s: assertion failed\n", __LINE__, #cond); \
> >          exit (1); \
> >      } \
> > } while (0)
> >
> > #define MY_ASSERT_ERRNO(cond) do { \
> >      if (!(cond)) { \
> >          fprintf (stderr, "%d: %s: %m\n", __LINE__, #cond); \
> >          exit (1); \
> >      } \
> > } while (0)
> >
> > static void
> > mount_debugfs (void)
> > {
> >      if (mount (NULL, "/tmp/debugfs", "debugfs", 0, NULL) != 0)
> >          {
> >              perror ("mount debugfs");
> >              exit (1);
> >          }
> > }
> >
> > int
> > main (void)
> > {
> >      MY_ASSERT_ERRNO (chdir ("/") == 0);
> >      MY_ASSERT_ERRNO (unshare (CLONE_NEWNS) == 0);
> >      MY_ASSERT_ERRNO (mount (NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL) == 0);
> >      MY_ASSERT_ERRNO (mount (NULL, "/tmp", "tmpfs", 0, NULL) == 0);
> >      MY_ASSERT_ERRNO (mkdir ("/tmp/debugfs", 0777) == 0);
> >      mount_debugfs ();
> >      MY_ASSERT_ERRNO (mount (NULL, "/tmp/debugfs/tracing", "tmpfs", 0, NULL) == 0);
> >      execlp ("/bin/busybox", "sh", NULL);
> >      MY_ASSERT (false);
> > }
> >
>