Date: Tue, 13 Dec 2022 14:19:39 -0800 From: Rick Macklem <rick.macklem@gmail.com> To: FreeBSD CURRENT <freebsd-current@freebsd.org> Subject: What to do about a few lines in vfs_domount() never executed? Message-ID: <CAM5tNy7DM__tj4CQ0L_-ugo5krmEvLJgbU-WFLs47MrRBuGaNQ@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
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
[-- Attachment #2 --]
<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-family:monospace">Hi,</div><div class="gmail_default" style="font-family:monospace"><br></div><div class="gmail_default" style="font-family:monospace">While working on getting mountd/nfsd to run in a vnet</div><div class="gmail_default" style="font-family:monospace">prison, I came across the following lines near the</div><div class="gmail_default" style="font-family:monospace">beginning of vfs_domount() in sys/kern/vfs_mount.c:</div><div class="gmail_default" style="font-family:monospace"><br></div><div class="gmail_default" style="font-family:monospace">if (fsflags & MNT_EXPORTED) {</div><div class="gmail_default" style="font-family:monospace"> error = priv_check(td, PRIV_VFS_MOUNT_EXPORTED);</div><div class="gmail_default" style="font-family:monospace"> if (error)</div><div class="gmail_default" style="font-family:monospace"> return (error);</div><div class="gmail_default" style="font-family:monospace">}</div><div class="gmail_default" style="font-family:monospace"><br></div><div class="gmail_default" style="font-family:monospace">#1 - Since MNT_EXPORTED is never set in fsflags, this code never</div><div class="gmail_default" style="font-family:monospace"> gets executed.</div><div class="gmail_default" style="font-family:monospace"> --> I am asking what to do with the above code, since that</div><div class="gmail_default" style="font-family:monospace"> changes for the patch that allows mountd to run in a vnet</div><div class="gmail_default" style="font-family:monospace"> prison.</div><div class="gmail_default" style="font-family:monospace">#2 - priv_check(td, PRIV_VFS_MOUNT_EXPORTED) always returns 0</div><div class="gmail_default" style="font-family:monospace"> because nothing in sys/kern/kern_priv.c checks</div><div class="gmail_default" style="font-family:monospace"> PRIV_VFS_MOUNT_EXPORTED.</div><div class="gmail_default" style="font-family:monospace"><br></div><div class="gmail_default" style="font-family:monospace">I don't know what the original author's thinking was w.r.t. this.</div><div class="gmail_default" style="font-family:monospace">Setting exports already checks that the mount operation can be</div><div class="gmail_default" style="font-family:monospace">done by the requestor.</div><div class="gmail_default" style="font-family:monospace"><br></div><div class="gmail_default" style="font-family:monospace">So, what do you think should be done with the above code snippet?</div><div class="gmail_default" style="font-family:monospace">- Consider it cruft and delete it.</div><div class="gmail_default" style="font-family:monospace">- Try and figure out what PRIV_VFS_MOUNT_EXPORTED should check?</div><div class="gmail_default" style="font-family:monospace">- Leave it as is. After the patch that allows mountd to run in</div><div class="gmail_default" style="font-family:monospace"> a vnet prison, MNT_EXPORTED will be set in fsflags, but the</div><div class="gmail_default" style="font-family:monospace"> priv_check() call will just return 0. (A little overhead,</div><div class="gmail_default" style="font-family:monospace"> but otherwise no semantics change.)</div><div class="gmail_default" style="font-family:monospace"><br></div><div class="gmail_default" style="font-family:monospace">rick</div><div class="gmail_default" style="font-family:monospace"><br></div></div></div>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAM5tNy7DM__tj4CQ0L_-ugo5krmEvLJgbU-WFLs47MrRBuGaNQ>