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

See Also:


Attachments
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
media
[root@]~# tree mnt
mnt
`-- media
Comment 1 Pedro F. Giffuni freebsd_committer freebsd_triage 2018-01-16 04:12:13 UTC
Assign it to -fs list.
Comment 2 Colin Percival freebsd_committer freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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:

https://reviews.freebsd.org/D27832
Comment 8 Martin Birgmeier 2020-12-30 11:53:15 UTC
*** Bug 246496 has been marked as a duplicate of this bug. ***
Comment 9 commit-hook freebsd_committer freebsd_triage 2021-02-17 10:09:54 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=63640b2f552c0476f50484635eb9888eafcd22dc

commit 63640b2f552c0476f50484635eb9888eafcd22dc
Author:     Robert Wing <rew@FreeBSD.org>
AuthorDate: 2021-02-17 09:22:23 +0000
Commit:     Robert Wing <rew@FreeBSD.org>
CommitDate: 2021-02-17 10:02:56 +0000

    automount(8): fix absolute path when creating a mountpoint

    When executing automount(8), it will attempt to create the directory where an
    autofs filesystem is to be mounted. Explicity set the root path for this
    directory to "/".

    This fixes the issue where the directory being created was being treated as a
    relative path instead of an absolute path (as expected).

    PR:     224601
    Reported by:    kusumi.tomohiro@gmail.com
    Reviewed by:    trasz
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D27832

 usr.sbin/autofs/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 10 commit-hook freebsd_committer freebsd_triage 2021-02-25 18:56:45 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=cf9829d98dc771f9ca0696e493dc3bb635999536

commit cf9829d98dc771f9ca0696e493dc3bb635999536
Author:     Robert Wing <rew@FreeBSD.org>
AuthorDate: 2021-02-17 09:22:23 +0000
Commit:     Robert Wing <rew@FreeBSD.org>
CommitDate: 2021-02-25 18:53:02 +0000

    automount(8): fix absolute path when creating a mountpoint

    When executing automount(8), it will attempt to create the directory where an
    autofs filesystem is to be mounted. Explicity set the root path for this
    directory to "/".

    This fixes the issue where the directory being created was being treated as a
    relative path instead of an absolute path (as expected).

    PR:     224601
    Reported by:    kusumi.tomohiro@gmail.com
    Reviewed by:    trasz
    Differential Revision:  https://reviews.freebsd.org/D27832

    (cherry picked from commit 63640b2f552c0476f50484635eb9888eafcd22dc)

 usr.sbin/autofs/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 11 commit-hook freebsd_committer freebsd_triage 2021-02-25 18:58:48 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=d9e70f5d97c63ae5dba93e9b026d1cfa1b1a4759

commit d9e70f5d97c63ae5dba93e9b026d1cfa1b1a4759
Author:     Robert Wing <rew@FreeBSD.org>
AuthorDate: 2021-02-17 09:22:23 +0000
Commit:     Robert Wing <rew@FreeBSD.org>
CommitDate: 2021-02-25 18:57:37 +0000

    automount(8): fix absolute path when creating a mountpoint

    When executing automount(8), it will attempt to create the directory where an
    autofs filesystem is to be mounted. Explicity set the root path for this
    directory to "/".

    This fixes the issue where the directory being created was being treated as a
    relative path instead of an absolute path (as expected).

    PR:     224601
    Reported by:    kusumi.tomohiro@gmail.com
    Reviewed by:    trasz
    Differential Revision:  https://reviews.freebsd.org/D27832

    (cherry picked from commit 63640b2f552c0476f50484635eb9888eafcd22dc)

 usr.sbin/autofs/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)