Page MenuHomeFreeBSD

vfs_exports: Tighten bounds and assert consistency of numsecflavors
ClosedPublic

Authored by freqlabs on May 7 2020, 10:33 PM.
Tags
None
Referenced Files
F101953838: D24753.diff
Tue, Nov 5, 6:40 PM
Unknown Object (File)
Wed, Oct 9, 3:23 PM
Unknown Object (File)
Mon, Oct 7, 12:13 AM
Unknown Object (File)
Oct 1 2024, 2:40 AM
Unknown Object (File)
Oct 1 2024, 2:22 AM
Unknown Object (File)
Oct 1 2024, 2:18 AM
Unknown Object (File)
Oct 1 2024, 12:17 AM
Unknown Object (File)
Sep 21 2024, 5:28 PM
Subscribers

Details

Summary

We know the value must be greater than 0 and less than MAXSECFLAVORS.

Reject values outside this range in the initial check in vfs_export and add KASSERTs
in the later consumers.

Also check that we are called with one of either MNT_DELEXPORT or MNT_EXPORTED set.

Sponsored by: iXsystems, Inc.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

sys/kern/vfs_export.c
308

This is actually a problem for MNT_DELEXPORT, where we don't use ex_numsecflavors and it is 0.

After you have updated your patch, I'll look again.
Thanks for doing this, rick

sys/kern/vfs_export.c
120

It might be better to make this a check that returns an
error instead of a KASSERT(). Although this can only
be done by "root", I'm not sure panicing because a
userland daemon does something bogus is what I'd do.
(You can printf(), so that the bogus case gets logged.)

308

Yes, allowing 0 for the MNT_DELEXPORT case is correct.
I'd either just leave this code or add a check for MNT_DELEXPORT
and allow the 0 for that case, but not for MNT_EXPORTED.

sys/kern/vfs_export.c
120

There should be no way for these asserts to fail, due to the check in vfs_export. Likewise for the KASSERTs in vfs_stdcheckexp. We should be able to assert these invariants, and should an assertion fail, something really has gone terribly wrong.

308

I'll add a check that either MNT_DELEXPORT or MNT_EXPORTED is set, and when MNT_EXPORTED is set do the additional bounds check.

freqlabs edited the summary of this revision. (Show Details)
rmacklem added inline comments.
sys/kern/vfs_export.c
120

Yes, you are correct. I was thinking vfs_hang_addrlist() was being called from
other places than after the sanity checks.

This revision is now accepted and ready to land.May 10 2020, 10:18 PM