Bug 274600

Comment 1 John Baldwin 2023-10-23 16:44:42 UTC

Please see the proposed patch at https://reviews.freebsd.org/D42327

Comment 2 commit-hook 2023-11-18 19:09:48 UTC

A commit in branch main references this bug:

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

commit 3eed4803f943e2937325e81140b88e2e8eea8deb
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2023-11-18 19:08:34 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2023-11-18 19:08:34 +0000

    vfs mount: Consistently use ENODEV internally for an invalid fstype

    Change vfs_byname_kld to always return an error value of ENODEV to
    indicate an unsupported fstype leaving ENOENT to indicate errors such
    as a missing mount point or invalid path.  This allows nmount(2) to
    better distinguish these cases and avoid treating a missing device
    node as an invalid fstype after commit 6e8272f317b8.

    While here, change mount(2) to return EINVAL instead of ENODEV for an
    invalid fstype to match nmount(2).

    PR:             274600
    Reviewed by:    pstef, markj
    Differential Revision:  https://reviews.freebsd.org/D42327

 sys/kern/vfs_init.c  | 4 +++-
 sys/kern/vfs_mount.c | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

Was any consideration given to the vfs.usermount=1 case (that is, non-root users)? That is the case were keeping EPERM from vfs_byname_kld seems appropriate. (A distinct error message would be appreciated, though.)

Comment 4 John Baldwin 2023-11-28 20:51:45 UTC

That was not a case I considered, no.  Is the case that kern_kldload fails with EPERM?

Here is how it looks like on 13.2:
% cd
% mkdir umnt
% mount -t nullfs /etc umnt
mount_nullfs: /usr/home/alex/umnt: Operation not permitted
% sudo kldload nullfs
% mount -t nullfs /etc umnt
% mount | grep umnt
/etc on /usr/home/alex/umnt (nullfs, local, noatime, nosuid, nfsv4acls, mounted by alex)

In general, as far as I can tell, there are 4 types of permission
checks non-root users care about: vfs.usermount sysctl, PRIV_KLD_LOAD,
additional fs-specific PRIV_VFS_MOUNT_whatever checks [1],
directory permissions. None of them has distinct error messages.

Well, the commit in question changes one of them to "Invalid fstype",
which is how I found this PR.

[1]: https://github.com/freebsd/freebsd-src/blob/5d4f897f88ed87d3dc4204fbfac726eac5472001/sys/fs/tarfs/tarfs_vfsops.c#L976

Comment 6 commit-hook 2024-01-03 20:33:45 UTC

A commit in branch stable/14 references this bug:

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

commit b636e35bb724b9c777fed6c7cf4a713d14316f36
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2023-11-18 19:08:34 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2024-01-03 20:18:44 +0000

    vfs mount: Consistently use ENODEV internally for an invalid fstype

    Change vfs_byname_kld to always return an error value of ENODEV to
    indicate an unsupported fstype leaving ENOENT to indicate errors such
    as a missing mount point or invalid path.  This allows nmount(2) to
    better distinguish these cases and avoid treating a missing device
    node as an invalid fstype after commit 6e8272f317b8.

    While here, change mount(2) to return EINVAL instead of ENODEV for an
    invalid fstype to match nmount(2).

    PR:             274600
    Reviewed by:    pstef, markj
    Differential Revision:  https://reviews.freebsd.org/D42327

    (cherry picked from commit 3eed4803f943e2937325e81140b88e2e8eea8deb)

 sys/kern/vfs_init.c  | 4 +++-
 sys/kern/vfs_mount.c | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

Comment 7 John Baldwin 2024-01-22 03:11:07 UTC

The original bug was not committed to stable/13, so no MFC to 13 is warranted.  I've left this open only because I want to fix the EPERM issue for the usermount case still.