cred: groupmember(): Remove tolerance for empty 'cr_groups'
This tolerance had been introduced in commit caa309c8811d ("nfsd: Fix
handling of credentials with cr_ngroups == 0", on 2024/10/21).
Now that NFS has been modified in the previous commit (cfbe7a62dc62,
"nfs, rpc: Ensure kernel credentials have at least one group") to rule
out credentials with empty 'cr_groups' (and thus, no 'cr_gid'), remove
it back.
This basically reverts the above-mentioned commit except for the not
directly related modifications it contains, which are still valid.
Discussed with: rmacklem (by mail)
Approved by: markj (mentor)
MFC after: 3 days
nfs, rpc: Ensure kernel credentials have at least one group
This fixes several bugs where some 'struct ucred' in the kernel,
constructed from user input (via nmount(2)) or obtained from other
servers (e.g., gssd(8)), could have an unfilled 'cr_groups' field and
whose 'cr_groups[0]' (or 'cr_gid', which is an alias) was later
accessed, causing an uninitialized access giving random access rights.
Use crsetgroups_fallback() to enforce a fallback group when possible.
For NFS, the chosen fallback group is that of the NFS server in the
current VNET (NFSD_VNET(nfsrv_defaultgid)).
There does not seem to be any sensible fallback available in rpc code
(sys/rpc/svc_auth.c, svc_getcred()) on AUTH_UNIX (TLS or not), so just
fail credential retrieval there. Stock NSS sources, rpc.tlsservd(8) or
rpc.tlsclntd(8) provide non-empty group lists, so will not be impacted.
Discussed with: rmacklem (by mail)
Approved by: markj (mentor)
[2 lines not shown]
cred: groupmember() and co.: Sanity check cred's groups (INVARIANTS)
Leverage the normalization check functions introduced in the previous
commit in all public-facing groups search functions to catch programming
errors early.
Approved by: markj (mentor)
MFC after: 3 days
cred: New crsetgroups_fallback()
Similar to crsetgroups(), but allows an empty group array in input,
treating it like a one-element array containing the passed fallback
group.
Approved by: markj (mentor)
MFC after: 3 days
Differential Revision: https://reviews.freebsd.org/D46917
cred: crextend(): Harden, simplify
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).
Reviewed by: mhorne (older version)
Approved by: markj (mentor)
MFC after: 3 days
Differential Revision: https://reviews.freebsd.org/D46915
cred: crsetgroups(): Improve and factor out groups normalization
The groups array has been sorted (not including the first element, which
is always the effective GID) to enable performing a binary search for
determining if some group is part of the supplementary groups set.
Factor out this sorting operation into an internal normalization
function (groups_normalize()), adding to it the removal of duplicates
after the sort.
Separating groups normalization code allows to perform it in advance,
and in particular before calling MAC hooks which need the groups array
to be sorted to perform better. This also enables sorting input arrays
ahead of acquiring the process lock (which is not necessary for this
operation).
kern_setgroups() has been changed accordingly, so MAC modules
implementing the mac_cred_check_setgroups() hook now can assume
a normalized groups array (and also that it has at least one element, as
[28 lines not shown]
cred: 'kern.ngroups' tunable: Limit it to avoid internal overflows
As the comment introduced with the tunable said (but the code didn't
do), make sure that 'ngroups_max' can't be INT_MAX, as this would cause
overflow in the usual 'ngroups_max + 1' computations (as we store the
effective GID and supplementary groups' IDs in the same array, and
'ngroups_max' only applies to supplementary groups).
Further, we limit the maximum number of groups somewhat arbitrarily to
~17M so as to avoid overflow when computing the size in bytes of the
groups set's backing array and to avoid obvious configuration errors.
We really don't think that more than ~17M groups will ever be needed (if
I'm proven wrong one day, please drop me a note about your use case).
While here, document more precisely why NGROUPS_MAX needs to be the
minimum value for 'ngroups_max'.
Reviewed by: mhorne (older version)
Approved by: markj (mentor)
[2 lines not shown]
cred: kern_setgroups(): Internally use int as number of groups' type
sys_setgroups() (and sys_getgroups()) was changed in commit "kern: fail
getgroup and setgroup with negative int" (4bc2174a1b48) to take the
number of groups as an 'int' (for sys_getgroups(), POSIX mandates this
change; for sys_setgroups(), which it does not standardize, it's
arguably for consistency).
All our internal APIs related to groups on 'struct ucred', as well as
related members on the latter, treat that number as an 'int' as well
(and not a 'u_int').
Consequently, to avoid surprises, change kern_setgroups() to behave the
same, and fix audit_arg_groupset() accordingly. With that change,
everything is handled with signed integers internally.
Update sanity checks accordingly.
Reviewed by: mhorne
[3 lines not shown]
cred: Separate constant for the number of inlined groups
CRED_SMALLGROUPS_NB now holds the number of inlined groups in field
'cr_smallgroups'. XU_NGROUPS stays the number of groups allowed in
'struct xucred'. The first is an implementation detail, while the
second is part of a public interface. All mentions of XU_NGROUPS in the
tree have been reviewed and only those concerning the implementation
detail have been changed to use CRED_SMALLGROUPS_NB (they were all in
'kern_prot.c').
No functional change, as CRED_SMALLGROUPS_NB is set to 16, the same
value as XU_NGROUPS.
Reviewed by: mhorne (slightly different version)
Approved by: markj (mentor)
MFC after: 3 days
Differential Revision: https://reviews.freebsd.org/D46911
cred: group_is_supplementary(): Use bsearch()
This makes that function use a more efficient version of binary search
instead, and removes one more hand-rolled binary search code from the
tree (and the kernel binary).
Reviewed by: mhorne, emaste
Approved by: markj (mentor)
MFC after: 3 days
Differential Revision: https://reviews.freebsd.org/D46907
cred: supplementary_group_member() => group_is_supplementary()
The new name seems more immediately descriptive.
While here, constify its signature ('struct ucred' isn't modified).
While here, replace "supplemental" => "supplementary" in some comments.
No functional change (intended).
Reviewed by: mhorne, emaste
Approved by: markj (mentor)
MFC after: 3 days
Differential Revision: https://reviews.freebsd.org/D46906
Tentatively merge llvm fix for buildworld WITH_ASAN
Building world using WITH_ASAN results in an assertion when compiling
certain source files referencing ifuncs:
Assertion failed: (isa<Function>(Callee) || isa<GlobalAlias>(Callee)), function analyzeAllUses, file /root/freebsd/contrib/llvm-project/llvm/lib/Analysis/StackSafetyAnalysis.cpp, line 514.
This was already reported upstream a while ago, in
<https://github.com/llvm/llvm-project/issues/87923>, but now there is
finally a candidate fix, which seems trivial so I am importing it right
away.
Reported by: markj
PR: 280936
Pull Request: https://github.com/llvm/llvm-project/pull/113841
MFC after: 3 days
(cherry picked from commit f3457ed94241be9d4c2c3ab337c9086d5c45c43f)
Tentatively merge llvm fix for buildworld WITH_ASAN
Building world using WITH_ASAN results in an assertion when compiling
certain source files referencing ifuncs:
Assertion failed: (isa<Function>(Callee) || isa<GlobalAlias>(Callee)), function analyzeAllUses, file /root/freebsd/contrib/llvm-project/llvm/lib/Analysis/StackSafetyAnalysis.cpp, line 514.
This was already reported upstream a while ago, in
<https://github.com/llvm/llvm-project/issues/87923>, but now there is
finally a candidate fix, which seems trivial so I am importing it right
away.
Reported by: markj
PR: 280936
Pull Request: https://github.com/llvm/llvm-project/pull/113841
MFC after: 3 days
(cherry picked from commit f3457ed94241be9d4c2c3ab337c9086d5c45c43f)
freebsd-update: improve pkgbase check
The previous version used a case-insensitive match (default for -x).
The presence of packages like freebsd-git-devtools and freebsd-ftpd
would falsely trigger the packaged base check.
Instead, just use `pkg which /usr/bin/uname` as a packaged base
indication. pkg uses /usr/bin/uname to determine ABI, so we can rely on
it existing. If it comes from a package then packaged base is in use.
Also, extend the check to all freebsd-update commands. It is easier to
just disallow all commands, and easier to test.
Reported by: Mark Millard
Reviewed by: manu
Fixes: cf1aba2857c1 ("freebsd-update: refuse to operate on a pkgbase system")
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47378
cam: report sct/sc in that order
This should have no effect on scripting, but when reading it, sct/sc is
the natural order of things (and the order we print them in
elsewhere). Report them in that order.
Sponsored by: Netflix
newbus: Introduce bus_get_pass() and hide bus_current_pass
There's no reason to write to bus_current_pass outside of the controlled
times subr_bus.c does it, so move to an accessor and make
bus_current_pass private to newbus.
Reviewed by: imp
Pull Request: https://github.com/freebsd/freebsd-src/pull/1457
sctp: don't consider the interface name when removing an address
Checking the interface name can not be done consistently, so
don't do it.
MFC after: 3 days
syscons: add deprecation notice
syscons(4) is not compatible with UEFI, does not support UTF-8, and is
Giant-locked. There is no specific timeline yet for removing it, but
support for the Giant lock is expected to go away in one or two major
release cycles. Add a deprecation notice to avoid surprises, and help
ensure that any material deficiencies in vt(4) become known.
Reviewed by: manu, markj, imp
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47396
vt: add comments for KDMKTONE ioctl implementation
The KDMKTONE ioctl, introduced in commit 916347f77e6c, is used to beep
the PC speaker. For historical reasons the frequency is specified as an
8254 PIT divisor for a 1.19MHz clock. Linux provides this same ioctl.
Add a comment to vtterm_beep to avoid someone wanting to "fix" this in
the future.
Also add an XXX comment that the period unit is supposed to be "timer
ticks." Note that nothing in the base system uses this ioctl.
Reviewed by: imp
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D47395
igc: Remove a bogus register write in igc_if_queues_free()
As explained in PR 277038, iflib calls IFDI_DETACH() and then
IFDI_QUEUES_FREE(). With igc, the latter writes to a register after it
has been unmapped.
igc_if_detach() already calls igc_release_hw_control(), and looking at
callers of igc_if_queues_free(), that appears to be sufficient. So,
just remove the igc_release_hw_control() call.
PR: 277038
Reported by: Mike Belanger <mibelanger at qnx.com>
Reviewed by: kbowling
Tested by: kbowling
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D47293
(cherry picked from commit 35d05a14ed7e9935be1ed0fe965b91aaaa4c92ef)
igc: Remove a bogus register write in igc_if_queues_free()
As explained in PR 277038, iflib calls IFDI_DETACH() and then
IFDI_QUEUES_FREE(). With igc, the latter writes to a register after it
has been unmapped.
igc_if_detach() already calls igc_release_hw_control(), and looking at
callers of igc_if_queues_free(), that appears to be sufficient. So,
just remove the igc_release_hw_control() call.
PR: 277038
Reported by: Mike Belanger <mibelanger at qnx.com>
Reviewed by: kbowling
Tested by: kbowling
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D47293
(cherry picked from commit 35d05a14ed7e9935be1ed0fe965b91aaaa4c92ef)
nvme: Clarify a comment
Expand the comment a little to make it clearer: Once we've restarted,
we're out of the resetting phase in our state machine. The controller
has actually been out of reset since we started issuing commands to it
earlier in the resetting phase.
Sponsored by: Netflix
nvmf: Deregister the post_sync eventhandler correctly during detach
Previously the handler was removed from the wrong eventhandler list.
Fixes: f46d4971b5af nvmf: Handle shutdowns more gracefully
Sponsored by: Chelsio Communications
nvmf_tcp: Correct padding calculation
PDU data alignment (PDA) isn't necessarily a power of 2, just a
multiple of 4, so use roundup() instead of roundup2() to compute the
PDU data offset (PDO).
Sponsored by: Chelsio Communications
libnvmf: Correctly set the controller and host PDA fields
The caller supplied PDU data alignment (PDA) field from
nvmf_association_params is the caller's restriction on data alignment
(so affects received PDUs), and the PDA value received from the other
end is the remote end's restriction (so affects transmitted PDUs).
I had these backwards so that if the remote end advertised a PDA it
was used as the receive PDA instead of the transmit PDA.
Sponsored by: Chelsio Communications
nvmf: Add sysctl nodes for each queue pair
These report the queue size, queue head, queue tail, and the number of
commands submitted.
Sponsored by: Chelsio Communications
