Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: Skip fuse FS by /usr/libexec/security
To:
Ingo Schwarze <schwarze@usta.de>
Cc:
"Kirill A. Korinsky" <kirill@korins.ky>, tech@openbsd.org
Date:
Sat, 18 May 2024 01:41:41 +0300

Download raw body.

Thread
On Wed, May 15, 2024 at 11:07:29PM +0200, Ingo Schwarze wrote:
> Hi,
> 
> Kirill A. Korinsky wrote on Wed, May 15, 2024 at 08:05:51PM +0100:
>  
> > The better approach to solve the same issue were suggested [1]
> > [1]  https://marc.info/?t=171465050600003&r=1&w=2
> > and merged already.
> > 
> > So, no need to mvoe forward with this one.
> 
> I see, thanks for the pointer.
> 
> While looking at this, i noticed the related documentation appears
> to be a bit sparse.  So if anybody more familiar than me with the mount(2)
> system call wants to have a look and provide patches or direction
> for improving the manuals, that might be useful.
> 
>  * The mount(8) manual says
>      If no arguments are given to mount, this list is printed.
>    But it says nothing about the format of the list.
>    While it isn't hard to guess that the format is
>      special on node type type (options)
>    the "options" part can include "local" and the precise
>    meaning of that option appears to be undocumented.
> 
>  * According to "man -k any=MNT_LOCAL", the only page mentioning
>    MNT_LOCAL is df(1), which seems suspicious for two reasons:
>    Refering to a #define'd constant in a section 1 manual may be
>    excessive technicality, and not mentioning a #define'd constant
>    in section 2 or 3 may amount to a documentation gap, in particular
>    when the constant is so important that even non-programmer users
>    are expected to consider it.
> 
>  * The only APIs i can find that expose MNT_LOCAL to userland
>    are statfs(2), getfsstat(2), and fhstatfs(2) - but don't take
>    my word for it, i'm not quite familiar with this area.
>    None of these three manual pages mentions MNT_LOCAL.
> 
> One final observation, concerning kernel code and not only documentation:
> kern/vfs_init.c explicitly sets MNT_LOCAL on the MOUNT_FUSEFS entry
> in vfsconflist[], while it does not do so for MOUNT_NFS.
> Shortly after sys_mount() retrieves that entry with vfs_byname()
> and vfs_mount_alloc() copies it to mp->mnt_flag, fusefs_mount()
> in miscfs/fuse/fuse_vfsops.c unconditionally wipes it out saying:
> 
> 	/* FUSE file system is not truly local. */
> 	mp->mnt_flag &= ~MNT_LOCAL;
> 
> That looks somewhat confusing to me, but it may well just be me;
> there may be reasons for setting the flag in the top level VFS
> configuration and then only clearing it when the real work begins,
> i don't really have an idea...
> 

I was confused with explicitly setting MNT_LOCAL on `mnt_flag' which
seems to be not required. However, it is enough to drop MNT_LOCAL in
corresponding `vfsconflist' entry.

ok?

Index: sys/kern/vfs_init.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_init.c,v
diff -u -p -r1.43 vfs_init.c
--- sys/kern/vfs_init.c	26 Dec 2019 13:30:54 -0000	1.43
+++ sys/kern/vfs_init.c	17 May 2024 22:25:20 -0000
@@ -92,7 +92,7 @@ static struct vfsconf vfsconflist[] = {
 #endif
 
 #ifdef FUSE
-	{ &fusefs_vfsops, MOUNT_FUSEFS, 18, 0, MNT_LOCAL,
+	{ &fusefs_vfsops, MOUNT_FUSEFS, 18, 0, 0,
 	    sizeof(struct fusefs_args) },
 #endif
 
Index: sys/miscfs/fuse/fuse_vfsops.c
===================================================================
RCS file: /cvs/src/sys/miscfs/fuse/fuse_vfsops.c,v
diff -u -p -r1.46 fuse_vfsops.c
--- sys/miscfs/fuse/fuse_vfsops.c	7 May 2024 14:27:11 -0000	1.46
+++ sys/miscfs/fuse/fuse_vfsops.c	17 May 2024 22:25:20 -0000
@@ -114,8 +114,6 @@ fusefs_mount(struct mount *mp, const cha
 	fmp->allow_other = args->allow_other;
 
 	mp->mnt_data = fmp;
-	/* FUSE file system is not truly local. */
-	mp->mnt_flag &= ~MNT_LOCAL;
 	vfs_getnewfsid(mp);
 
 	memset(mp->mnt_stat.f_mntonname, 0, MNAMELEN);