Bug 282995

Summary:

mountd doesn't limit export with -alldirs

Product:

Base System

Reporter:

Michael Proto <mike>

Component:

misc

Assignee:

Rick Macklem <rmacklem>

Status:

Closed FIXED

Severity:

Affects Some People

CC:

emaste, pat, rb, rmacklem

Priority:

---

Flags:

rmacklem: mfc-stable14+

Version:

14.2-RELEASE

Hardware:

Any

OS:

Any

Attachments:

Description	Flags
Make mountd check for a mount point when -alldirs is specified	none

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

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

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

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

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

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!

Comment 10 commit-hook freebsd_committer

2025-01-19 23:55:13 UTC

A commit in branch stable/14 references this bug:

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

commit ead3cd3ef628ab11e814eea7c673eb6407f96c55
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2024-12-28 21:24:51 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2025-01-19 23:52:48 +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

    (cherry picked from commit 07cd69e272da2f116950f2bde6dfcc404f869f0c)

 usr.sbin/mountd/mountd.c | 51 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 18 deletions(-)

Comment 11 commit-hook freebsd_committer

2025-01-19 23:59:15 UTC

A commit in branch stable/14 references this bug:

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

commit 3ac1af1c0f07dbef049d42830732979ee35b3618
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2024-12-28 21:30:56 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2025-01-19 23:58:13 +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

    (cherry picked from commit 6db916d21a09aebf3e7cb5c95e2ad714ab2b8882)

 usr.sbin/mountd/mountd.8 | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

Comment 12 commit-hook freebsd_committer

2025-01-20 00:02:17 UTC

A commit in branch stable/14 references this bug:

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

commit 2cd9a4f0c229626f7422187b30723772336e8896
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2024-12-28 21:51:08 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2025-01-20 00:00:35 +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

    (cherry picked from commit 295934eaa92cd917ae42a446899c0d527ad9c0c9)

 usr.sbin/mountd/exports.5 | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

Comment 13 Rick Macklem freebsd_committer

2025-01-20 00:21:03 UTC

Patch for "-a" has been committed and MFC'd.