Re: stricter sys_mount() flag handling
On Mon, Oct 03, 2016 at 12:16:49PM +0200, Martin Natano wrote: > > /* > > * Flags set by internal operations. > > */ > > #define MNT_LOCAL 0x1000 /* filesystem is stored locally */ > > #define MNT_QUOTA 0x2000 /* quotas are enabled on filesystem > > */ > > #define MNT_ROOTFS 0x4000 /* identifies the root filesystem */ > > Yes, done. I also removed MNT_DOOMED and MNT_WANTRDWR from the flag > mask, as those flags are internal to the kernel and shouldn't be passed > from userland. I have just checked the mountd/mountd.c code. It does a statfs(path, ) and then mount(sfb.f_fstypename, sfb.f_mntonname, sfb.f_flags | MNT_UPDATE, ). So all flags that are set by the kernel can be passed back. But the kernel ignores some flags. The current diff breaks that semantics. The optnames in sbin/mount/mount.c contains a good list of flags that sould be ignored. The o_optname column is "". I still want the list of valid flags at the beginning of the system call and not hidden in all the file system specific implementations. So perhaps we can combine the your orignal diff with all flags and the clearing the internal flags like it is done in sys_unmount. If the bits are not defined anywhere, return EINVAL. Then clear all bits that should not be set by mount or unmount system call. > Now, that MNT_FLAGMASK doesn't contain all flags anymore, I replaced the > magic constant with the flag names to make it clearer which flags are > included. I am not sure wether we should define MNT_FLAGMASK in the header or only in the system call implementation. > +#define MNT_FLAGMASK (MNT_RDONLY|MNT_SYNCHRONOUS|MNT_NOEXEC|MNT_NOSUID|\ > + MNT_NODEV|MNT_NOPERM|MNT_ASYNC|MNT_WXALLOWED|\ > + MNT_EXRDONLY|MNT_EXPORTED|MNT_DEFEXPORTED|\ > + MNT_EXPORTANON|MNT_NOATIME|MNT_UPDATE|MNT_DELEXPORT|\ > + MNT_RELOAD|MNT_FORCE|MNT_SOFTDEP) I think the nfs export flags can only be set in the ufs_args export_info.ex_flags field. bluhm
Re: stricter sys_mount() flag handling
> > And I want this check also for sys_unmount(). > > > > Good idea, sys_mount() is easy, because the only flag allowed there is > MNT_FORCE. s/sys_mount/sys_unmount/
Re: stricter sys_mount() flag handling
On Sat, Oct 01, 2016 at 06:01:34PM +0200, Martin Natano wrote:
> After committing the new MNT_NOPERM flag I got some complaints that my
> code doesn't work by people that recompiled mount_ffs, but didn't reboot
> to the new kernel. I don't blame them; in that situation sys_mount()
> silently ignores the unknown flag. IMHO we should check the flags more
> strictly. Ok?
I think we once had a simmilar problem, when someone tried to unmount
with MNT_DOOMED. So I like to check all flags at the beginning of
the system call.
But I think you should remove these from the mask:
/*
* Flags set by internal operations.
*/
#define MNT_LOCAL 0x1000 /* filesystem is stored locally */
#define MNT_QUOTA 0x2000 /* quotas are enabled on filesystem */
#define MNT_ROOTFS 0x4000 /* identifies the root filesystem */
And I want this check also for sys_unmount().
bluhm
> Index: sys/mount.h
> ===
> RCS file: /cvs/src/sys/sys/mount.h,v
> retrieving revision 1.127
> diff -u -p -r1.127 mount.h
> --- sys/mount.h 10 Sep 2016 16:53:30 - 1.127
> +++ sys/mount.h 1 Oct 2016 15:36:11 -
> @@ -414,6 +414,11 @@ struct mount {
> #define MNT_DOOMED 0x0800 /* device behind filesystem is gone */
>
> /*
> + * All mount flags.
> + */
> +#define MNT_FLAGMASK0x0e0f
> +
> +/*
> * Flags for various system call interfaces.
> *
> * waitfor flags to vfs_sync() and getfsstat()
> Index: kern/vfs_syscalls.c
> ===
> RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
> retrieving revision 1.265
> diff -u -p -r1.265 vfs_syscalls.c
> --- kern/vfs_syscalls.c 10 Sep 2016 16:53:30 - 1.265
> +++ kern/vfs_syscalls.c 1 Oct 2016 15:36:11 -
> @@ -117,6 +117,9 @@ sys_mount(struct proc *p, void *v, regis
> if ((error = suser(p, 0)))
> return (error);
>
> + if (flags & ~MNT_FLAGMASK)
> + return (EINVAL);
> +
> /*
>* Mount points must fit in MNAMELEN, not MAXPATHLEN.
>*/
stricter sys_mount() flag handling
After committing the new MNT_NOPERM flag I got some complaints that my
code doesn't work by people that recompiled mount_ffs, but didn't reboot
to the new kernel. I don't blame them; in that situation sys_mount()
silently ignores the unknown flag. IMHO we should check the flags more
strictly. Ok?
natano
Index: sys/mount.h
===
RCS file: /cvs/src/sys/sys/mount.h,v
retrieving revision 1.127
diff -u -p -r1.127 mount.h
--- sys/mount.h 10 Sep 2016 16:53:30 - 1.127
+++ sys/mount.h 1 Oct 2016 15:36:11 -
@@ -414,6 +414,11 @@ struct mount {
#define MNT_DOOMED 0x0800 /* device behind filesystem is gone */
/*
+ * All mount flags.
+ */
+#defineMNT_FLAGMASK0x0e0f
+
+/*
* Flags for various system call interfaces.
*
* waitfor flags to vfs_sync() and getfsstat()
Index: kern/vfs_syscalls.c
===
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.265
diff -u -p -r1.265 vfs_syscalls.c
--- kern/vfs_syscalls.c 10 Sep 2016 16:53:30 - 1.265
+++ kern/vfs_syscalls.c 1 Oct 2016 15:36:11 -
@@ -117,6 +117,9 @@ sys_mount(struct proc *p, void *v, regis
if ((error = suser(p, 0)))
return (error);
+ if (flags & ~MNT_FLAGMASK)
+ return (EINVAL);
+
/*
* Mount points must fit in MNAMELEN, not MAXPATHLEN.
*/
