Re: What to do about a few lines in vfs_domount() never executed?
If you're 100.0% percent sure that it will never be run, you can
delete it. But if you're only 99.9%, then I suggest converting it
into a KASSERT.
On Tue, Dec 13, 2022 at 3:20 PM 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
>
What to do about a few lines in vfs_domount() never executed?
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
Re: Any Cronyx Tau* (ce(4) or cp(4)) users ?
On Tue, Dec 13, 2022 at 02:02:05PM -0500, Ed Maste wrote: E> > So I'm fine with removal [of ce, cp drivers] if anybody demonstrates E> > me a non-zero cost of leaving the drivers in. E> E> I just found PR264160: likely heap overflow in driver for Cronyx E> [ce(4), cp(4)] adapters (others also possibly affected) E> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=264160 E> although it was submitted some time ago. I am not sure if it's a real E> issue or not. At this point I'm fine with removing the drivers. -- Gleb Smirnoff
Re: Any Cronyx Tau* (ce(4) or cp(4)) users ?
On Thu, 16 Dec 2021 at 11:49, Gleb Smirnoff wrote: > > So I'm fine with removal [of ce, cp drivers] if anybody demonstrates > me a non-zero cost of leaving the drivers in. I just found PR264160: likely heap overflow in driver for Cronyx [ce(4), cp(4)] adapters (others also possibly affected) https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=264160 although it was submitted some time ago. I am not sure if it's a real issue or not.
