iflib: Do not hold the ifnet lock across registration
iflib_device_register() acquired IFNET_WLOCK to preserve lock order
when ether_ifattach() was called with the context lock held. The context
lock is now released around ether_ifattach(), making registration-wide
ifnet serialization unnecessary.
Keeping IFNET_WLOCK across driver attachment also allows synchronous
interface event handlers to recurse on it. The rtnetlink interface-group
dump does so through if_foreach_group() while handling the interface
attachment event.
Remove the outer lock and the corresponding failure-path unlock and
relock transitions. Continue to drop the context lock around
ether_ifattach() and taskqueue drains, and preserve context-lock coverage
for driver attach and detach.
Validated under WITNESS on 82576 and I226 controllers. Multiple VF
attach and detach cycles, netmap control operations, and every iflib
[12 lines not shown]
syslogd: Pipes need the CAP_PDGETPID right as well
While here, use caph_rights_limit(), as syslogd already uses
caph_enter().
PR: 298104
Reported by: mi
Fixes: 24816abb8740 ("syslogd: Limit rights on procdescs")
MFC after: 3 days
syslogd/tests: Amend a test to catch leaked process descriptors
This serves to catch the regression fixed by commit
1a669b66ddb4 ("syslogd: reap pipe children on config reload").
MFC after: 1 week
dtrace/tests: compile D sources at runtime on test target
Previously, we would precompile D test dependencies using the
host's dtrace, which unconditionally outputs ELF files in the
host's format. This breaks the cross-compile build with errors
like the following:
dtrace: failed to link script: incorrect ELF machine type for
object file: tst.usdt.pieo
--- usdt.o ---
*** Failed target: usdt.o
This patch moves compilation to runtime for all C-based testcases
that have a dependent D source file.
Reviewed by: markj
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D59030
devstat: Fix a kernel stack disclosure
The 16-byte "device_name" field was not zero-filled, so could contain
uninitialized stack data. Zero the whole struct, as that's the
prevailing pattern for this kind of conversion code, and it's more
robust in the face of future revisions to struct devstat.
Reviewed by: olce, kib
Reported by: Reo Shiseki
Fixes: a11d132f6c62 ("devstat: Provide 32-bit compatibility")
MFC after: 3 days
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D59309
(cherry picked from commit 7cb1a76f88158fb690418336b736e66c238cd4f7)
mtw: fix zero-length queue array that can corrupt struct mtw_softc
The mtw softc declares sc_epq with MTW_BULK_RX even though MTW_BULK_RX is enum
value 0, while initialization and queue handling index up to MTW_EP_QUEUES;
attaching a matching USB WLAN device can drive writes past the absent array and
corrupt adjacent softc fields.
This suggested patch sizes sc_epq with MTW_EP_QUEUES so the softc contains the
endpoint queues the driver initializes and uses.
Fixes: c14b01624261 ("mt7601U: Importing if_mtw from OpenBSD")
Reviewed by: bz
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D58897
(cherry picked from commit 7e9e72bee359437b9f78c6a4056ef0a90337f341)
malloc: Use ckdint.h helpers instead of WOULD_OVERFLOW
This serves to demonstrate some usage of the ckdint.h helpers. The new
version also generates better machine code on amd64 and arm64.
Reviewed by: kib, emaste
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
(cherry picked from commit 2d67765f10e7da43ba2d4a7fc074c15d5354684b)
sys: Add sys/ckdint.h
We have a C23 stdckdint.h header for userspace, which provides checked
addition, subtraction and multiplication. We lack similar helpers in
the kernel, where they are regularly needed.
Let's just adopt the C23 macros. For bonus points, I added a wrapper to
ensure that ignored an return value is raised as an error by the
compiler.
Reviewed by: kib, emaste
MFC after: 2 weeks
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D58773
(cherry picked from commit 37bd69d43c70346b9191f7ce07ee9ed783ce528f)
rsu: add a runtime TX buffer bound check for a kernel buffer overflow
The rsu driver currently relies on a `KASSERT` to prove that the mbuf payload
plus TX descriptor fits in the per-transfer USB TX buffer. On production
kernels without `INVARIANTS`, an oversized raw 802.11 frame can reach
`m_copydata()` and overwrite past that buffer, causing local kernel memory
corruption.
This suggested patch replaces the assertion-only guard with a runtime size
check before the copy. Oversized frames return `EMSGSIZE`, leaving the existing
caller cleanup paths responsible for freeing `m0`, `ni`, and the unused
transfer buffer.
Reachable via root / bpf access
Reviewed by: bz, adrian
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D58898
(cherry picked from commit 81a67bfebc60055bbf19ce6e39537fb5f53eeee5)
amd64: Mark the trapframe as initialized in ipi_bitmap_handler()
Fixes: fdc1f3450634 ("x86: change signatures of ipi_{bitmap,swi}_handler() to take pointer")
MFC after: 1 week
Sponsored by: The FreeBSD Foundation
(cherry picked from commit b566e0a7232b4989e79df95c2f02bdedef3ac4e7)
tools/build: stage stdckdint.h's dependencies for non-FreeBSD hosts
37bd69d43c7 gave stdckdint.h two new includes, <sys/_visible.h> and
<sys/ckdint.h>. Neither reaches a non-FreeBSD host: _visible.h is
staged only under ${.MAKE.OS} == "FreeBSD" and ckdint.h is not staged at
all, so the libc bootstrap fails on reallocarray.o when cross-building
from macOS. Both headers are self-contained; stage them alongside
stdckdint.h.
Fixes: 37bd69d43c7 ("sys: Add sys/ckdint.h")
Reviewed by: rpaulo, markj
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D58943
(cherry picked from commit 805c5004fa86e26486175cd3a0bb253dcb8ec7e0)
tools/build: Bring in stdckdint.h if needed
This is needed when bootstrapping libc, reallocarray.c and
recallocarray.c include stdckdint.h now.
Reviewed by: emaste
Fixes: 7233893e9496 ("lib{c,openbsd}: use ckd_mul() for overflow checking in re(c)allocarray")
Differential Revision: https://reviews.freebsd.org/D52932
(cherry picked from commit 687cb66411c7bc220ccb90cedc2f7486567d55b6)
devstat: Fix a kernel stack disclosure
The 16-byte "device_name" field was not zero-filled, so could contain
uninitialized stack data. Zero the whole struct, as that's the
prevailing pattern for this kind of conversion code, and it's more
robust in the face of future revisions to struct devstat.
Reviewed by: olce, kib
Reported by: Reo Shiseki
Fixes: a11d132f6c62 ("devstat: Provide 32-bit compatibility")
MFC after: 3 days
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D59309
(cherry picked from commit 7cb1a76f88158fb690418336b736e66c238cd4f7)
iflib: Require sustained demand for TX watchdog
The restored watchdog arms when the outstanding descriptor count
grows, but then continues counting based only on the queue remaining
frozen. A single growth sample can therefore leave a quiet, nearly
empty queue armed until the watchdog resets the interface. Lockless
sampling of the queue counters can also manufacture the initial growth
sample.
This matches watchdog reports from I354 queues with 979 or 980 of
1022 usable descriptors still available. Neither queue was under
transmit backpressure when the reset flapped its link.
Keep the watchdog armed only while the outstanding count continues
to grow, the software ring is stalled, or the hardware ring is at
iflib's backpressure threshold. The last condition preserves hang
detection with simple-TX, which bypasses the software ring. A busy hang
still reaches the verdict while a frozen but quiet tail disarms. Retain
the final driver completion peek so a missed completion interrupt
[14 lines not shown]
vmimage.subr: Use makefs -N
By default, makefs uses the host environment's user and group databases
when creating filesystems. This causes makefs to fail when trying to
create files owned by users or groups which don't exist in the host
environment, for example when creating a VM with packages pre-installed
which added their own users/groups.
Pass "-N ${DESTDIR}/etc" to makefs to point it at the user and group
databases from the image being created.
MFC after: 1 week
Sponsored by: Amazon
oci: Install FreeBSD-pam in the runtime image
In 16.0/15.1, the PAM modules were split from FreeBSD-runtime into a
new FreeBSD-pam package. FreeBSD-runtime does not install FreeBSD-pam,
which means if a user starts from runtime, then installs sshd, sshd will
fail to authenticate users because of missing PAM modules.
Since FreeBSD-pam is relatively small (about 230kB on amd64), and is
already part of FreeBSD-set-minimal, add it to the runtime image as
well. Users who absolutely don't want this can still build their
own images without it.
MFC after: 1 week
Reviewed by: dfr
Reported by: Michael Johnson <ahze at ahze.net>
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D59194
syslogd: reap pipe children on config reload
On SIGHUP reload, closelogfiles() frees each F_PIPE filed even when its
pipe process is still running. close_filed() sets f_type to F_UNUSED
before the check, so the condition f_type != F_PIPE is always true and
the filed is freed while its process descriptor is still on the dead
queue and registered in the kqueue. When the child later exits, the
NOTE_EXIT handler dereferences the freed filed (use-after-free) and
never closes the process descriptor, leaving the pipe child as a
persistent zombie.
Capture whether the filed is a pipe with an active process descriptor
before calling close_filed(), and defer the free in that case so the
NOTE_EXIT handler can reap the child and free the filed.
Reviewed by: markj
Fixes: 95381c0139d6 (syslogd: Use process descriptors)
Differential Revision: https://reviews.freebsd.org/D59319
hwpmc(4): stop overwalking past _start on amd64 user callchains
pmc_save_user_callchain() emits the pc it just loaded before checking
whether fp is the ABI's zero frame-chain terminator. At the bottom of
a well-formed chain under _start, fp comes back 0 as expected, but the
paired pc is stale rtld data left on the stack -- a legal userspace VA
that still passes PMC_IN_USERSPACE(), so it gets emitted as a bogus
extra frame. This shows up in flame graphs as a spurious hex-valued
root frame below _start.
Check fp == 0 alongside the existing checks before emitting, matching
how arm/arm64/powerpc already load the next fp before their check.
Measured via 1kHz hwpmc sampling on an OCA: stacks with any unresolved
hex frame drop from 23.9% to 1.3%, and stacks with hex at the root drop
from 5.5% to 0.3%.
Reviewed by: mhorne, Ali Mashtizadeh <ali at mashtizadeh.com>, gallatin
MFC after: 3 days
[2 lines not shown]
ipfilter: Avoid negative array indicies
Array indices must always be posive. We avoid this by making each index
unsigned. This mitigates out-of-bounds reads and writes.
Reported by: Ilja Van Sprundel <ivansprundel at ioactive.com>
Reviewed by: glebius
Differential revision: https://reviews.freebsd.org/D55260
(cherry picked from commit 3fdbd8a07a2dcb8fe3cec19fc59ef064453e4755)
ipfilter: Avoid negative array indicies
Array indices must always be posive. We avoid this by making each index
unsigned. This mitigates out-of-bounds reads and writes.
Reported by: Ilja Van Sprundel <ivansprundel at ioactive.com>
Reviewed by: glebius
Differential revision: https://reviews.freebsd.org/D55260
(cherry picked from commit 3fdbd8a07a2dcb8fe3cec19fc59ef064453e4755)
sys/conf/options: Add WITNESS_LOCK_CHILDCOUNT
Make the witness LOCK_CHILDCOUNT a configurable kernel option.
On machines with a very high core count the default value is too
low, leading to witness exhaustion after boot.
Relnotes: yes
Reviewed by: kib, ziaee
Signed-off-by: Kajetan Puchalski <kajetan.puchalski at arm.com>
Closes: https://github.com/freebsd/freebsd-src/pull/2398
devstat: Fix a kernel stack disclosure
The 16-byte "device_name" field was not zero-filled, so could contain
uninitialized stack data. Zero the whole struct, as that's the
prevailing pattern for this kind of conversion code, and it's more
robust in the face of future revisions to struct devstat.
Reviewed by: olce, kib
Reported by: Reo Shiseki
Fixes: a11d132f6c62 ("devstat: Provide 32-bit compatibility")
MFC after: 3 days
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.freebsd.org/D59309
bcm2835_audio: Comment out vchi_service_release()
bcm2835_audio_release() calls vchi_service_close() and then
unconditionally calls vchi_service_release() with the same service
handle.
In the VCHI shim implementation, a successful vchi_service_close() calls
service_free(service). The subsequent vchi_service_release() therefore
dereferences a freed SHIM_SERVICE_T object when it reads
service->handle, resulting in a use-after-free panic.
vchi_service_release(), however, releases a reference which might block
vchi_service_close() from completing successfuly, so comment it out
instead of removing it altogether, until further testing is done.
PR: 297187
MFC after: 2 weeks
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D58921
[2 lines not shown]
