Bug 282995 - mountd doesn't limit export with -alldirs
Summary: mountd doesn't limit export with -alldirs
Status: In Progress
Alias: None
Product: Base System
Classification: Unclassified
Component: misc (show other bugs)
Version: 14.2-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: Rick Macklem
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-11-27 03:08 UTC by Michael Proto
Modified: 2024-12-29 01:41 UTC (History)
4 users (show)

See Also:
rmacklem: mfc-stable14?


Attachments
Make mountd check for a mount point when -alldirs is specified (2.36 KB, patch)
2024-11-27 22:03 UTC, Rick Macklem
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Proto 2024-11-27 03:08:08 UTC
Found this while testing some NFS export operations, initially reported https://lists.freebsd.org/archives/freebsd-stable/2024-November/002531.html


mountd doesn't limit exports to a filesystem mountpoint when -alldirs is enabled. Per exports(5):

EXAMPLES
...
          /cdrom -alldirs,quiet,ro -network 192.168.33.0 -mask 255.255.255.0

...
     The file system rooted at /cdrom will be exported read-only to the entire
     network 192.168.33.0/24, including all its subdirectories.  Since /cdrom
     is the conventional mountpoint for a CD-ROM device, this export will fail
     if no CD-ROM medium is currently mounted there since that line would then
     attempt to export a subdirectory of the root file system with the
     -alldirs option which is not allowed.  The -quiet option will then
     suppress the error message for this condition that would normally be
     syslogged.  As soon as an actual CD-ROM is going to be mounted, mount(8)
     will notify mountd(8) about this situation, and the /cdrom file system
     will be exported as intended.  Note that without using the -alldirs
     option, the export would always succeed.  While there is no CD-ROM medium
     mounted under /cdrom, it would export the (normally empty) directory
     /cdrom of the root file system instead.




mountd is currnetly not working as indicated. If /cdrom is exported via mountd and is not at the time a filesystem mount-point, the root filesystem (/) is exported instead with the mountd warning:

Nov 20 22:34:56 zfstest1 mountd[27724]: Warning: exporting /cdrom
exports entire / file system


More details provided via the lists link above
Comment 1 Rick Macklem freebsd_committer freebsd_triage 2024-11-27 22:03:19 UTC
Created attachment 255500 [details]
Make mountd check for a mount point when -alldirs is specified

This patch adds a check that the directory path
in the exports line is a mount point when the
-alldirs export option is specified.

This semantic has not been enforced since releng1,
but is documented in the Examples section of exports(5).

I am waiting for feedback on freebsd-current@ w.r.t.
how I should proceed w.r.t. fixing this.

If the reporter can test this patch, that would be appreciated.
Comment 2 Michael Proto 2024-11-28 03:41:03 UTC
Looks good on my 14.2-RC1 test system with the patch applied and mountd rebuilt:


  /etc/exports:
/cdrom		-alldirs,quiet,ro -network 10.0.0.0/24

(/cdrom is not currently a mountpoint)

# killall -HUP mountd
# tail /var/log/messages
Nov 27 22:36:37 <daemon.err> zfstest1 mountd[13985]: alldirs for non-fs root dir
Nov 27 22:36:37 <daemon.err> zfstest1 mountd[13985]: bad exports list line '/cdrom-alldirs'


(from a clinet):
$ sudo mount -t nfs -r oddjob:/cdrom /mnt
[tcp] oddjob:/cdrom: Permission denied


I can test on a 14.1-p6 systems as well if needed, but this looks like a winner.
Comment 3 Michael Proto 2024-11-28 03:43:57 UTC
On the host (zfstest1/oddjob), 'showmount -e' also lists no exported filesystems
Comment 4 Michael Proto 2024-11-28 03:54:34 UTC
When /cdrom is mounted on the host, the mount works as expected/anticipated:

# mdconfig -a -t vnode -f /tmp/FreeBSD-14.1-RELEASE-amd64-disc1.iso
# mount -r -t cd9660 /dev/md0 /cdrom
# mount | tail -1
/dev/md0 on /cdrom (cd9660, NFS exported, local, read-only)
# showmount -e
Exports list on localhost:
/cdrom                             10.0.0.0

(on a client):
# mount -t nfs -r oddjob:/cdrom /mnt
# mount | tail -n1
oddjob:/cdrom on /mnt (nfs, read-only)



Donn't know if this is expected but after I unmount /cdrom on the host, 'showmount -e' still shows /cdrom as exported (but I do get the same "permission denied" when trying to mount from the client, and no NFS mount).
Sending a HUP to mountd and running a 'showmount -e' again not reports no exported filesystems.
Comment 5 commit-hook freebsd_committer freebsd_triage 2024-12-28 21:26:54 UTC
A commit in branch main references this bug:

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

commit 07cd69e272da2f116950f2bde6dfcc404f869f0c
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2024-12-28 21:24:51 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2024-12-28 21:24:51 +0000

    mountd.c: Define a new -a command line option

    Bugzilla PR#282995 reported that, when a file system was
    exported with the "-alldirs" flag, the export succeeded even
    if the directory path was not a server file system mount point.

    This behaviour for "-alldirs" was only documented in the
    Example section of exports(5) and had not been enforced
    since FreeBSD2. (A patch applied between FreeBSD1 and
    FreeBSD2 broke the check for file system mount point.)

    Since the behaviour of allowing the export has existed since
    FreeBSD2, the concensus on a mailing list was that it would
    be a POLA violation to change it now.
    Therefore, this patch adds a new "-a" mountd command line
    option to enforce a check for the exported directory being a
    server file system mount point.

    PR:     282995
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D48137

 usr.sbin/mountd/mountd.c | 51 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 18 deletions(-)
Comment 6 commit-hook freebsd_committer freebsd_triage 2024-12-28 21:32:58 UTC
A commit in branch main references this bug:

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

commit 6db916d21a09aebf3e7cb5c95e2ad714ab2b8882
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2024-12-28 21:30:56 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2024-12-28 21:30:56 +0000

    mound.8: Document the new -a command line option

    Commit 07cd69e272da adds a new "-a" mountd option.

    This patch updates the man page for it.

    This is a content change.

    PR:     282995
    Reviewed by:    gbe (manpages)
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D48138

 usr.sbin/mountd/mountd.8 | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)
Comment 7 commit-hook freebsd_committer freebsd_triage 2024-12-28 21:53:01 UTC
A commit in branch main references this bug:

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

commit 295934eaa92cd917ae42a446899c0d527ad9c0c9
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2024-12-28 21:51:08 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2024-12-28 21:51:08 +0000

    exports.5: Document the current behavior of -alldirs

    Commit 07cd69e272da adds a new "-a" mountd option,
    which changes the behavior of mountd when file systems
    are exported via -alldirs.

    This patch updates the man page to reflect the actual
    behavior when -alldirs is used when mountd is started
    with/without -a.  Prior to the above commit, exports(5)
    documented that, when -alldirs was specified, the exports
    line would fail unless the directory was a server file
    system mount point.  This behavior was only documented
    in the Examples section and has not been implemented
    since a change between FreeBSD 1 and FreeBSD 2 was done.

    This is a contents change.

    PR:     282995
    Reviewed by:    markj
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D48139

 usr.sbin/mountd/exports.5 | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)
Comment 8 Rick Macklem freebsd_committer freebsd_triage 2024-12-28 22:01:38 UTC
The patch has now been committed to main.
Sorry, I misses a Tested by: Michael Proto <mike@jellydonut.org>
on the commit.
Comment 9 Michael Proto 2024-12-29 01:41:36 UTC
Thank you Rick for your investigation and fix for this issue. It is very appreciated!