Date: Mon, 17 Dec 2018 09:01:34 -0700 From: Alan Somers <asomers@freebsd.org> Cc: jamie@freebsd.org, src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org>, Ross Williams <ross@ross-williams.net> Subject: Re: svn commit: r333263 - in head: lib/libjail sys/cddl/contrib/opensolaris/uts/common/fs/zfs sys/compat/linprocfs sys/compat/linsysfs sys/fs/devfs sys/fs/fdescfs sys/fs/nullfs sys/fs/procfs sys/fs/pse... Message-ID: <CAOtMX2jwZzTvUp0dQ_Nm27CLxG4fwf1M9nFDR7mtXHUS0E88yQ@mail.gmail.com> In-Reply-To: <CAOtMX2gtnRq38TAThgNRtk7EPJknvoTfQK6x9yBu28FWJH_7LA@mail.gmail.com> References: <201805042054.w44KsRtc038808@repo.freebsd.org> <CAOtMX2jBiyRm_bZ%2B9OohqZXK%2Bxq--q24p4MWpjudRc=HKQUmrg@mail.gmail.com> <f379b6c84ad858d39ec96bf470f928b2@freebsd.org> <CAOtMX2jEKAVJNYX2mJt9FnB=VngaWB=%2BwDXSg2hFD6rO7bFG4A@mail.gmail.com> <c9cc4b86158ff55c12e10da94de95127@freebsd.org> <CAOtMX2gtnRq38TAThgNRtk7EPJknvoTfQK6x9yBu28FWJH_7LA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Nov 19, 2018 at 10:26 AM Alan Somers <asomers@freebsd.org> wrote: > > On Fri, Nov 16, 2018 at 7:16 PM James Gritton <jamie@freebsd.org> wrote: >> >> On 2018-11-16 16:30, Alan Somers wrote: >> >> On Fri, Nov 16, 2018 at 2:28 PM James Gritton <jamie@freebsd.org> wrote: >>> >>> On 2018-11-16 10:34, Alan Somers wrote: >>> >>> On Fri, May 4, 2018 at 2:54 PM Jamie Gritton <jamie@freebsd.org> wrote: >>>> >>>> Author: jamie >>>> Date: Fri May 4 20:54:27 2018 >>>> New Revision: 333263 >>>> URL: https://svnweb.freebsd.org/changeset/base/333263 >>>> >>>> Log: >>>> Make it easier for filesystems to count themselves as jail-enabled, >>>> by doing most of the work in a new function prison_add_vfs in kern_j= ail.c >>>> Now a jail-enabled filesystem need only mark itself with VFCF_JAIL, = and >>>> the rest is taken care of. This includes adding a jail parameter li= ke >>>> allow.mount.foofs, and a sysctl like security.jail.mount_foofs_allow= ed. >>>> Both of these used to be a static list of known filesystems, with >>>> predefined permission bits. >>>> >>>> Reviewed by: kib >>>> Differential Revision: D14681 >>>> >>>> Modified: >>>> head/lib/libjail/jail.c >>>> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c >>>> head/sys/compat/linprocfs/linprocfs.c >>>> head/sys/compat/linsysfs/linsysfs.c >>>> head/sys/fs/devfs/devfs_vfsops.c >>>> head/sys/fs/fdescfs/fdesc_vfsops.c >>>> head/sys/fs/nullfs/null_vfsops.c >>>> head/sys/fs/procfs/procfs.c >>>> head/sys/fs/pseudofs/pseudofs.h >>>> head/sys/fs/tmpfs/tmpfs_vfsops.c >>>> head/sys/kern/kern_jail.c >>>> head/sys/kern/vfs_init.c >>>> head/sys/kern/vfs_mount.c >>>> head/sys/kern/vfs_subr.c >>>> head/sys/sys/jail.h >>>> head/sys/sys/mount.h >>>> head/usr.sbin/jail/jail.8 >>>> >>>> Modified: head/lib/libjail/jail.c >>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D >>>> --- head/lib/libjail/jail.c Fri May 4 20:38:26 2018 (r3332= 62) >>>> +++ head/lib/libjail/jail.c Fri May 4 20:54:27 2018 (r3332= 63) >>>> @@ -1048,7 +1048,13 @@ kldload_param(const char *name) >>>> else if (strcmp(name, "sysvmsg") =3D=3D 0 || strcmp(name, "sys= vsem") =3D=3D 0 || >>>> strcmp(name, "sysvshm") =3D=3D 0) >>>> kl =3D kldload(name); >>>> - else { >>>> + else if (strncmp(name, "allow.mount.", 12) =3D=3D 0) { >>>> + /* Load the matching filesystem */ >>>> + kl =3D kldload(name + 12); >>>> + if (kl < 0 && errno =3D=3D ENOENT && >>>> + strncmp(name + 12, "no", 2) =3D=3D 0) >>>> + kl =3D kldload(name + 14); >>>> + } else { >>>> errno =3D ENOENT; >>>> return (-1); >>>> } >>> >>> I'm curious about this part of the change. Why is it necessary to load= the module in the "allow.mount.noXXXfs" case, when the jail is forbidden t= o mount the filesystem? It seems like that would just load modules that are= n't going to be used. >>> Additional discussion at https://github.com/iocage/iocage/issues/689 . >>> -Alan >>> >>> Presumably such a parameter would be included in some jails in conjunct= ion with the positive being included in others (perhaps as a default). The= truth is I never really considered whether the "no" option would be used, = I just always treat these option as pairs. >>> It may be reasonable (at least in the allow.mount.* case) to silently d= isregard a "no" option that doesn't exist, but I don't know how many places= would need to be modified for that to go smoothly. Though I don't expect = that there would be too many people who bother to include a jail parameter = about a filesystem which they're not planning to use. >>> - Jamie >> >> >> Well, many people use the "no" option because one of the most popular ja= il managers, iocage, uses it under the hood. But since "no" is the default= , its presence on the command line is a noop. Are there any situations in = which the "no" option has an effect? The only two possibilities I could th= ink of were: >> >> 1) Somebody puts both the positive and negative options on the same comm= and line. From experiment, it seems like the last option takes effect. In= this case, the presence of the positive option would cause the kld to be l= oaded, regardless of the presence of the negative option. >> 2) When using hierarchical jails, it might make sense to use the positiv= e option for the outer jail and the negative option for the inner jail. Bu= t this would only be important if the inner jail inherited the outer jail's= parameters, which doesn't seem to be the case. >> >> So I can't think of any reason to continue to mount the kld for "no" opt= ions. Can you? >> >> >> 3) There's allow.mount.foofs as a global parameter, with some jails over= riding that with a jail-specific allow.mount.nofoofs. In that case, KLD lo= ading shouldn't be a problem as global parameters typically come first. >> >> It makes sense not to load a KLD for a "no" option, as long as that opti= on is then silently ignored. I wouldn't want it to error out with "unknown= parameter". > > > See also https://github.com/iocage/iocage/issues/689 This issue should be fixed, for iocage at least, by https://github.com/iocage/iocage/commit/3c38801335ed7c67505f0000b70c4b3febc= ddbc1 . -Alan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAOtMX2jwZzTvUp0dQ_Nm27CLxG4fwf1M9nFDR7mtXHUS0E88yQ>