From nobody Fri Dec 16 21:07:20 2022 X-Original-To: freebsd-current@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4NYhWT2zR4zZddr for ; Fri, 16 Dec 2022 21:07:33 +0000 (UTC) (envelope-from rick.macklem@gmail.com) Received: from mail-pl1-x629.google.com (mail-pl1-x629.google.com [IPv6:2607:f8b0:4864:20::629]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4NYhWS48YBz3xtV for ; Fri, 16 Dec 2022 21:07:32 +0000 (UTC) (envelope-from rick.macklem@gmail.com) Authentication-Results: mx1.freebsd.org; dkim=pass header.d=gmail.com header.s=20210112 header.b="Mm/89ufI"; spf=pass (mx1.freebsd.org: domain of rick.macklem@gmail.com designates 2607:f8b0:4864:20::629 as permitted sender) smtp.mailfrom=rick.macklem@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-pl1-x629.google.com with SMTP id l10so3496087plb.8 for ; Fri, 16 Dec 2022 13:07:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=CfAUDc7pHuMEMfDvij+TeDql4BbKQLaKyaccqSIGRVY=; b=Mm/89ufI/fGjqRNF04JlmFqAw/PFn0JSGwEy9s2PZqy25l0QcjK9NwB+hu4Hgopzrw u0JvhzOttifr0VrfL2avn3AAC+GDBYGQEbXKQ7RBL/+SsGlPgcYHh6vUyltiWM0VGr+b 6oZ7MwiPazLIgUOF7bTAY8bIISbTyyEr2uU72zWdivnX2kDDO+r+YTTA6YoRdlJlS6y0 lKL2IOv47IRmaubp+T41s/rTGk2uao7juHwgS7SX+5O6JQgBklcPM7UooFu5BUlHohfG aK4hFBqwo2ULLtrIJ8mexOtvy1NMFidUYB5CYIzV9o5U3QYID8P3v3Ue+t7a3va90hM8 TbuA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=CfAUDc7pHuMEMfDvij+TeDql4BbKQLaKyaccqSIGRVY=; b=4nmTC4w6vi/xYfqS6IjB67dMxBpSKwYMtc2Nv1e9VHWAtyIkx2BC2xRExkPcjE/4+q 7bah0PmMtxGuQbwRZD4l+SmFZqbN+PnZTrs8ecUhFRgWxuRII2t8WtFbIHpX82fwfIhi ErDYfcvEISfxmfp//gZ9MbZOaiZKIYZ1qK4o7buskHAJbUNOYO7fPvdnVWRX9uZad4ut a21s3xgbzbubRZfQMJKl+e8EPZ5BAOlBhytBFH3ZTbiTTpnpYbrhJDB6wBNEPKOkOKD/ 6k9/8PxKqmUV0Oy0fbxpRBol6sTI8nKRg6P85wHk0BodixOCWSxOoiJapn4G6I6Xllf/ QM4A== X-Gm-Message-State: AFqh2koH7yKfinpgr7AzZvKvQ3Mj8HMfwqctzkuUiiL4XgZ2cRWMXx3M WOTqsUxT9Xsr2lmnHSaWHf8nhnR2Ck+VoKRGiW5lkvGzCg== X-Google-Smtp-Source: AA0mqf5/pQcoEic58w7X8xBFgfeQgH7UDpEdjWek44EmSwOC7pr3YJ2t4QzI/6yvyo0o4vjRGPfQcF3DIhX5JHkULDw= X-Received: by 2002:a17:90a:7e0a:b0:219:661f:9916 with SMTP id i10-20020a17090a7e0a00b00219661f9916mr1367132pjl.200.1671224851196; Fri, 16 Dec 2022 13:07:31 -0800 (PST) List-Id: Discussions about the use of FreeBSD-current List-Archive: https://lists.freebsd.org/archives/freebsd-current List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-current@freebsd.org MIME-Version: 1.0 References: <20221215021431.d190e55ee911f5e94799f953@dec.sakura.ne.jp> In-Reply-To: <20221215021431.d190e55ee911f5e94799f953@dec.sakura.ne.jp> From: Rick Macklem Date: Fri, 16 Dec 2022 13:07:20 -0800 Message-ID: Subject: Re: What to do about a few lines in vfs_domount() never executed? To: Tomoaki AOKI Cc: freebsd-current@freebsd.org Content-Type: multipart/alternative; boundary="00000000000098712405eff85bb9" X-Spamd-Result: default: False [-3.00 / 15.00]; NEURAL_HAM_LONG(-1.00)[-1.000]; SUBJECT_ENDS_QUESTION(1.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_SHORT(-1.00)[-0.999]; DMARC_POLICY_ALLOW(-0.50)[gmail.com,none]; R_DKIM_ALLOW(-0.20)[gmail.com:s=20210112]; R_SPF_ALLOW(-0.20)[+ip6:2607:f8b0:4000::/36]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; PREVIOUSLY_DELIVERED(0.00)[freebsd-current@freebsd.org]; ARC_NA(0.00)[]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::629:from]; TO_MATCH_ENVRCPT_SOME(0.00)[]; RCVD_TLS_LAST(0.00)[]; TAGGED_FROM(0.00)[]; MLMMJ_DEST(0.00)[freebsd-current@freebsd.org]; DKIM_TRACE(0.00)[gmail.com:+]; MID_RHS_MATCH_FROMTLD(0.00)[]; FREEMAIL_FROM(0.00)[gmail.com]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim]; RCPT_COUNT_TWO(0.00)[2]; MIME_TRACE(0.00)[0:+,1:+,2:~]; FROM_EQ_ENVFROM(0.00)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; FREEMAIL_ENVFROM(0.00)[gmail.com]; RCVD_COUNT_TWO(0.00)[2] X-Rspamd-Queue-Id: 4NYhWS48YBz3xtV X-Spamd-Bar: -- X-ThisMailContainsUnwantedMimeParts: N --00000000000098712405eff85bb9 Content-Type: text/plain; charset="UTF-8" Just to provide closure on this, I just committed a patch to main (that will be MFC'd in 2 weeks) that sets MNT_EXPORTED when the "export" option is specified to nmount(2). This restores the "only root can do exports" semantics that appear to have been the case pre-r158857. Thanks everyone for your comments, rick On Wed, Dec 14, 2022 at 9:15 AM Tomoaki AOKI wrote: > Tracking the commits, it was originally introduced to > sys/kern/vfs_syscalls.c at r22521 [1][2] (Mon Feb 10 02:22:35 1997 by > dyson, submitted by hsu@freebsd.org) and later centralized into > sys/kern/vfs_mount.c at r99264 [2]. > > But according to the comment above the codes, maybe it would be > intended to block userland programs or ports FS modules setting > MNT_EXPORTED. > > If I'm not mis-understanding, it can be the case when > *vfs.usermount sysctl is non-zero, > *underlying FS (to be exported) allows it, and > *non-root user tries to mount the FS via NFS. > > > [1] > > https://svnweb.freebsd.org/base/head/sys/kern/vfs_syscalls.c?revision=22521&view=markup&pathrev=99264 > > [2] > > https://svnweb.freebsd.org/base/head/sys/kern/vfs_syscalls.c?r1=22520&r2=22521&pathrev=99264& > > [3] > > https://cgit.freebsd.org/src/commit/sys/kern/vfs_mount.c?id=2b4edb69f1ef62fc38b02ac22b0a3ac09e43fa77 > > > On Tue, 13 Dec 2022 14:19:39 -0800 > Rick Macklem wrote: > > > Hi, > > > > While working on getting mountd/nfsd to run in a vnet > > prison, I came across the following lines near the > > beginning of vfs_domount() in sys/kern/vfs_mount.c: > > > > if (fsflags & MNT_EXPORTED) { > > error = priv_check(td, PRIV_VFS_MOUNT_EXPORTED); > > if (error) > > return (error); > > } > > > > #1 - Since MNT_EXPORTED is never set in fsflags, this code never > > gets executed. > > --> I am asking what to do with the above code, since that > > changes for the patch that allows mountd to run in a vnet > > prison. > > #2 - priv_check(td, PRIV_VFS_MOUNT_EXPORTED) always returns 0 > > because nothing in sys/kern/kern_priv.c checks > > PRIV_VFS_MOUNT_EXPORTED. > > > > I don't know what the original author's thinking was w.r.t. this. > > Setting exports already checks that the mount operation can be > > done by the requestor. > > > > So, what do you think should be done with the above code snippet? > > - Consider it cruft and delete it. > > - Try and figure out what PRIV_VFS_MOUNT_EXPORTED should check? > > - Leave it as is. After the patch that allows mountd to run in > > a vnet prison, MNT_EXPORTED will be set in fsflags, but the > > priv_check() call will just return 0. (A little overhead, > > but otherwise no semantics change.) > > > > rick > > > -- > Tomoaki AOKI > > --00000000000098712405eff85bb9 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Just to provide closure on this, I just committed a patch to
main (that will be M= FC'd in 2 weeks) that sets MNT_EXPORTED
when the "export" option is sp= ecified to nmount(2).

This restores the "only root can do exports" semantics t= hat
appea= r to have been the case pre-r158857.

Thanks everyone for your comments, rick


=
On Wed, De= c 14, 2022 at 9:15 AM Tomoaki AOKI <junchoon@dec.sakura.ne.jp> wrote:
Tracking the commits, it was originally i= ntroduced to
sys/kern/vfs_syscalls.c at r22521 [1][2] (Mon Feb 10 02:22:35 1997 by
dyson, submitted by hs= u@freebsd.org) and later centralized into
sys/kern/vfs_mount.c at r99264 [2].

But according to the comment above the codes, maybe it would be
intended to block userland programs or ports FS modules setting
MNT_EXPORTED.

If I'm not mis-understanding, it can be the case when
=C2=A0*vfs.usermount sysctl is non-zero,
=C2=A0*underlying FS (to be exported) allows it, and
=C2=A0*non-root user tries to mount the FS via NFS.


[1]
https://svnweb.freebsd.org/base/head/sys/kern/vfs_syscalls.c= ?revision=3D22521&view=3Dmarkup&pathrev=3D99264

[2]
https://svnweb.freebsd.org/base/head/sys/kern/vfs_syscalls.c?r1= =3D22520&r2=3D22521&pathrev=3D99264&

[3]
https://cgit.freebsd.org/src/commit/sys/kern/vfs_mount.c?id=3D2b4edb69f1e= f62fc38b02ac22b0a3ac09e43fa77


On Tue, 13 Dec 2022 14:19:39 -0800
Rick Macklem <rick.macklem@gmail.com> wrote:

> Hi,
>
> While working on getting mountd/nfsd to run in a vnet
> prison, I came across the following lines near the
> beginning of vfs_domount() in sys/kern/vfs_mount.c:
>
> if (fsflags & MNT_EXPORTED) {
>=C2=A0 =C2=A0 =C2=A0 error =3D priv_check(td, PRIV_VFS_MOUNT_EXPORTED);=
>=C2=A0 =C2=A0 =C2=A0 if (error)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (error);
> }
>
> #1 - Since MNT_EXPORTED is never set in fsflags, this code never
>=C2=A0 =C2=A0 =C2=A0 gets executed.
>=C2=A0 =C2=A0 =C2=A0 --> I am asking what to do with the above code,= since that
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 changes for the patch that allows mo= untd to run in a vnet
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 prison.
> #2 - priv_check(td, PRIV_VFS_MOUNT_EXPORTED) always returns 0
>=C2=A0 =C2=A0 =C2=A0 because nothing in sys/kern/kern_priv.c checks
>=C2=A0 =C2=A0 =C2=A0 PRIV_VFS_MOUNT_EXPORTED.
>
> I don't know what the original author's thinking was w.r.t. th= is.
> Setting exports already checks that the mount operation can be
> done by the requestor.
>
> So, what do you think should be done with the above code snippet?
> - Consider it cruft and delete it.
> - Try and figure out what PRIV_VFS_MOUNT_EXPORTED should check?
> - Leave it as is. After the patch that allows mountd to run in
>=C2=A0 =C2=A0a vnet prison, MNT_EXPORTED will be set in fsflags, but th= e
>=C2=A0 =C2=A0priv_check() call will just return 0. (A little overhead,<= br> >=C2=A0 =C2=A0but otherwise no semantics change.)
>
> rick


--
Tomoaki AOKI=C2=A0 =C2=A0 <junchoon@dec.sakura.ne.jp>

--00000000000098712405eff85bb9--