From owner-svn-src-all@freebsd.org Mon Nov 19 17:26:33 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 154A3110DAE6; Mon, 19 Nov 2018 17:26:33 +0000 (UTC) (envelope-from asomers@gmail.com) Received: from mail-lj1-f169.google.com (mail-lj1-f169.google.com [209.85.208.169]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 3BCFD80C61; Mon, 19 Nov 2018 17:26:32 +0000 (UTC) (envelope-from asomers@gmail.com) Received: by mail-lj1-f169.google.com with SMTP id n18-v6so5877772lji.7; Mon, 19 Nov 2018 09:26:32 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OMUIucwwhZ4mInbMD/qmxtUPg0i6uh6NMlUgnwL4jKM=; b=uRPRtoip694t/+c3TBqP515dwplXxwd+73m4kacPJVWdXq2T6dxNlbKtgF8dD2EbrV vcmkDpYlsMP5ch3CVbQ6NweueF4drg9bw+2z3Qw0VvcjAZTMAutsESFUo/7UwJs32fdE fKb9BSikzKcwUoEiQQjTqV7O+eL4dK5UaRZxf5ttphaBaPjZAsLb2F0LDCBxH48+lJY1 E/syBswgY4CE0KVoLSp5htlmuby9XRJEcZPjX51HFPCAfFbXIP77uNAx5j8D8Ub2e1R1 ea9nomUlEvbfPiZks4l+mcVzPYINCr2Ur3DKZ0SM3Kei2m1CtrketGkfwuatX170Pk0t 7b/Q== X-Gm-Message-State: AGRZ1gJ2FnsVQ+rKKwUg76h/9kiabFZKiPXeqdcAgAE6ql3ZVb2sCECG bGppXZPdzLARNYCqHKeAKCQM/OtiXclGZpYjXh1VPYPO X-Google-Smtp-Source: AJdET5f5pBpq6otbScOkhqkVLwYUYE58pRaUqU5Urk49Zx/Qy2+wQMXPgIwvT+oju9Fpfyk45GcKdNWhdgvINGoy8wQ= X-Received: by 2002:a2e:5418:: with SMTP id i24-v6mr13233068ljb.51.1542648384803; Mon, 19 Nov 2018 09:26:24 -0800 (PST) MIME-Version: 1.0 References: <201805042054.w44KsRtc038808@repo.freebsd.org> In-Reply-To: From: Alan Somers Date: Mon, 19 Nov 2018 10:26:13 -0700 Message-ID: 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... To: jamie@freebsd.org Cc: src-committers , svn-src-all , svn-src-head , Ross Williams X-Rspamd-Queue-Id: 3BCFD80C61 X-Spamd-Result: default: False [-3.03 / 15.00]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-0.99)[-0.993,0]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; R_SPF_ALLOW(-0.20)[+ip4:209.85.128.0/17]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; RCVD_TLS_LAST(0.00)[]; DMARC_NA(0.00)[freebsd.org]; RCPT_COUNT_FIVE(0.00)[5]; TO_MATCH_ENVRCPT_SOME(0.00)[]; MX_GOOD(-0.01)[cached: alt3.gmail-smtp-in.l.google.com]; NEURAL_HAM_SHORT(-0.98)[-0.984,0]; RCVD_IN_DNSWL_NONE(0.00)[169.208.85.209.list.dnswl.org : 127.0.5.0]; IP_SCORE(-1.05)[ipnet: 209.85.128.0/17(-3.47), asn: 15169(-1.67), country: US(-0.09)]; FORGED_SENDER(0.30)[asomers@freebsd.org,asomers@gmail.com]; RWL_MAILSPIKE_POSSIBLE(0.00)[169.208.85.209.rep.mailspike.net : 127.0.0.17]; R_DKIM_NA(0.00)[]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US]; FROM_NEQ_ENVFROM(0.00)[asomers@freebsd.org,asomers@gmail.com]; RCVD_COUNT_TWO(0.00)[2] X-Rspamd-Server: mx1.freebsd.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Nov 2018 17:26:33 -0000 On Fri, Nov 16, 2018 at 7:16 PM James Gritton wrote: > On 2018-11-16 16:30, Alan Somers wrote: > > On Fri, Nov 16, 2018 at 2:28 PM James Gritton wrote: > >> On 2018-11-16 10:34, Alan Somers wrote: >> >> On Fri, May 4, 2018 at 2:54 PM Jamie Gritton 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_jail.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 like >>> allow.mount.foofs, and a sysctl like security.jail.mount_foofs_allowed. >>> 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 >>> >>> ============================================================================== >>> --- head/lib/libjail/jail.c Fri May 4 20:38:26 2018 (r333262) >>> +++ head/lib/libjail/jail.c Fri May 4 20:54:27 2018 (r333263) >>> @@ -1048,7 +1048,13 @@ kldload_param(const char *name) >>> else if (strcmp(name, "sysvmsg") == 0 || strcmp(name, "sysvsem") >>> == 0 || >>> strcmp(name, "sysvshm") == 0) >>> kl = kldload(name); >>> - else { >>> + else if (strncmp(name, "allow.mount.", 12) == 0) { >>> + /* Load the matching filesystem */ >>> + kl = kldload(name + 12); >>> + if (kl < 0 && errno == ENOENT && >>> + strncmp(name + 12, "no", 2) == 0) >>> + kl = kldload(name + 14); >>> + } else { >>> errno = 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 to >> mount the filesystem? It seems like that would just load modules that >> aren'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 >> conjunction 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 >> disregard 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 jail > 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 > think of were: > > 1) Somebody puts both the positive and negative options on the same > command 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 loaded, regardless of the presence of the negative option. > 2) When using hierarchical jails, it might make sense to use the positive > option for the outer jail and the negative option for the inner jail. But > 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" > options. Can you? > > > 3) There's allow.mount.foofs as a global parameter, with some jails > overriding that with a jail-specific allow.mount.nofoofs. In that case, > KLD loading 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 option > is then silently ignored. I wouldn't want it to error out with "unknown > parameter". > See also https://github.com/iocage/iocage/issues/689