Page MenuHomeFreeBSD

cred: crextend(): Harden, simplify
ClosedPublic

Authored by olce on Oct 4 2024, 8:07 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 24, 12:55 PM
Unknown Object (File)
Tue, Nov 19, 12:23 PM
Unknown Object (File)
Tue, Nov 19, 10:56 AM
Unknown Object (File)
Tue, Nov 19, 1:07 AM
Unknown Object (File)
Mon, Nov 18, 10:13 PM
Unknown Object (File)
Mon, Nov 18, 9:47 PM
Unknown Object (File)
Fri, Nov 15, 10:17 AM
Unknown Object (File)
Tue, Nov 12, 4:27 AM
Subscribers

Details

Summary

Harden by adding more assertions, and a plain panic in case of an
unrepresentable size for the groups array (this can never happen after
the change of the 'kern.ngroups' computation to impose some not too high
maximum value a few commits ago). Fix an impact in kern_setgroups().

Simplify by removing the iterative process whose purpose is actually to
determine the closest power of two that is greater than the wanted
number of bytes. Using the proper target quantity (number of bytes)
incidentally helps with eliminating divisions (and the reliance on
sizeof(gid_t) being a power of two).

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 60265
Build 57149: arc lint + arc unit

Event Timeline

olce requested review of this revision.Oct 4 2024, 8:07 AM
mhorne added inline comments.
sys/kern/kern_prot.c
2329

formally it should be size_t.

This revision is now accepted and ready to land.Oct 29 2024, 3:01 PM
olce added inline comments.
sys/kern/kern_prot.c
2329

Oh, you're right. The int comes from an initial version where the variable still held a number of groups as the original code, and I forgot to change the type when I switched to a number of bytes. Which makes me notice that an overflow check is missing (as in the original code), I'll add one after the nbytes computation, coupled with another change in D46913.

olce marked an inline comment as done.
olce edited the summary of this revision. (Show Details)

Switch nbytes to size_t, check for overflow with a plain panic (following D46913 latest change).

This revision now requires review to proceed.Oct 30 2024, 5:56 PM

Amend the comment in crcopy(), and add one in sys/cred.h to request to move the cr_ngroups field out of the copied area the next time a change of struct ucred's ABI is necessary.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 2 2024, 8:40 PM
This revision was automatically updated to reflect the committed changes.