Bug 224601 - autofs: Make automount(8) call chdir("/") before create_directory()
Summary: autofs: Make automount(8) call chdir("/") before create_directory()
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-fs (Nobody)
Keywords: patch
: 246496 (view as bug list)
Depends on:
Reported: 2017-12-26 18:06 UTC by kusumi.tomohiro
Modified: 2020-12-30 11:53 UTC (History)
5 users (show)

See Also:

Fixes the issue. (1.91 KB, application/mbox)
2017-12-26 18:06 UTC, kusumi.tomohiro
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description kusumi.tomohiro 2017-12-26 18:06:14 UTC
Created attachment 189116 [details]
Fixes the issue.

Unlike automountd where the daemon is daemonized or lesser-daemonized,
an automount process isn't necessarily at /, and this results in
creating unneeded directories at the current directory.

In the example below, mounting autofs on /mnt/media fails because the
command mkdirs mnt/media instead of /mnt/media. If /mnt/media already
exists the command can mount autofs on /mnt/media, but it still mkdirs
unneeded directories mnt/media.

Calling chdir("/") before creation and restoring the directory after
creation avoids this.

[root@]~# automount -L
/mnt/media                -nosuid               -media               # indirect map referenced at /etc/auto_master:8
[root@]~# ls mnt
ls: mnt: No such file or directory
[root@]~# automount
automount: cannot mount map -media on /mnt/media: No such file or directory
[root@]~# mount | grep autofs
[root@]~# ls mnt
[root@]~# tree mnt
`-- media
Comment 1 Pedro F. Giffuni freebsd_committer 2018-01-16 04:12:13 UTC
Assign it to -fs list.
Comment 2 Colin Percival freebsd_committer 2020-06-15 19:14:49 UTC
I just tripped over this too.  As an alternative to adding chdir("/") we could make create_directory take an "int absolute" parameter and anchor the paths being created in that case.

trasz, any preference of how you'd like this to be fixed?
Comment 3 Edward Tomasz Napierala freebsd_committer 2020-06-15 19:45:46 UTC
The create_directory() should always be passed an absolute path.  There's even the assert, in usr.sbin/autofs/common.c:create_directory():

assert(path[0] == '/');

Thus, my preferred fix would be to figure out how the path ends up being relative, and fixing that.
Comment 4 Colin Percival freebsd_committer 2020-06-15 19:55:31 UTC
That's easy then:

139	        /*
140	         * +1 to skip the leading slash.
141	         */
142	        copy = tofree = checked_strdup(path + 1);

We're not copying the leading slash for some reason.  Maybe because you didn't want to try to mkdir("/")?
Comment 5 Edward Tomasz Napierala freebsd_committer 2020-06-15 20:07:15 UTC
I think you're right regarding the leading slash.  I have no idea why it's like this, I'm afraid - it ignores EEXIST couple of lines below...
Comment 6 kusumi.tomohiro 2020-06-15 22:52:08 UTC
Can you commit the fix ?

I ported and maintain autofs in NetBSD and DragonFlyBSD, with the chdir("/") patch.
I'll replace it with above.
Comment 7 Robert Wing freebsd_committer 2020-12-30 00:55:59 UTC
The leading slash is ignored because otherwise strsep() will return '\0' on
first iteration.

concat() doesn't concatenate the separator when either of the strings passed in is empty. In this case, partial is empty on the first iteration, hence the separator not being included in the first pass.

There's a few different ways to tackle this, heres one with a bit more description:

Comment 8 Martin Birgmeier 2020-12-30 11:53:15 UTC
*** Bug 246496 has been marked as a duplicate of this bug. ***