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]
tests/sys/netinet/tcp_socket: fix build with ATF 0.22+
Confirm that creating clients/sockets was successful by testing the
result separate from the assignment and test that the return value is
not -1 instead of testing that the value returned is non-zero.
This fixes the build with [ATF 0.22+][1].
MFC after: 2 weeks
Reported by: clang (-Wparenthesis)
Reviewed by: tuexen, cc
Differential Revision: https://reviews.freebsd.org/D59284
[1]: https://github.com/freebsd/atf/pull/72
tests/sys/kern/ktls_test: fix -Wsign-compare issue
Cast the size_t quantity used in a comparison to off_t to mute a
`-Wsign-compare` complaint that now occurs after ATF 0.22 [1].
MFC after: 2 weeks
Reported by: clang
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D59285
[1]: https://github.com/freebsd/atf/pull/72
snd_uaudio: drop the default monitor level to 10%
As noted in the comment, some headsets with a hardware sidetone are
incredibly sensitive and emit immediate feedback upon attach with the
current system-wide default of 75%. Drop it down just for snd_uaudio(4)
to avoid incredibly unpleasant surprises.
MFC after: 3 days
Reviewed by: christos
Differential Revision: https://reviews.freebsd.org/D59199
rc: add a backlight service to save/restore backlight levels
The default on my laptop is annoyingly bright, and this is a useful
feature to mitigate that. The backlight script is largely a copy of the
mixer service which provides the same value for mixers, but this one is
specifically dependant on kld to allow DRM drivers a chance to attach.
Note that it's off by default to avoid interference with DEs, and
document the capability in backlight(8). Set backlight_enable=YES in
rc.conf(5) to enable save/restore.
Relnotes: maybe
Reviewed by: bapt, ivy, manu, ziaee
Differential Revision: https://reviews.freebsd.org/D59296
ufshci: skip the reinit when the new link works
UFSHCI_QUIRK_REINIT_AFTER_MAX_GEAR_SWITCH always rebuilt the
link after the gear switch. It threw away a working HS link and
ended up in PWM. The reinit is only needed for a dead link.
There the local side reports HS and the peer never answers. A
local readback cannot tell the two apart. Peer traffic can.
Probe the peer with DME_PEER_GET after the switch. Skip the
reinit when the probe succeeds. Log it when the probe fails.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D59299
ufshci: set HS series per platform and adapt type per gear
The driver always asked for Rate-B. It never set the adaptation
type. The Snapdragon X Elite firmware tunes the PHY for Rate-A.
A Rate-B link dies at every gear there. HS-G4 and above need
initial adaptation. This is a UniPro rule. It applies to
every host.
Add an hs_series field to the device tables. Use Rate-A on the
Snapdragon X Elite. Keep Rate-B on the PCI hosts. A table entry
without an HS series fails to attach. Set PA_TxHsAdaptType to
initial adaptation at HS-G4 and above. Leave it alone below
that. Hosts before UniPro 1.8 do not have it. The Galaxy Book
4 Edge now links at HS-G5 Rate-A.
fio results (128k sequential, 4k random, posixaio):
QD | SEQ_R(MiB/s) | SEQ_W(MiB/s) | RND_R(kIOPS) | RND_W(kIOPS)
----+--------------+--------------+--------------+-------------
[10 lines not shown]
ufshci: fix the Snapdragon X Elite reference clock
The driver's ACPI table set bRefClkFreq to 19.2 MHz. The
Snapdragon X Elite feeds the device 38.4 MHz from its CXO. The
firmware has no property for it. The device ran its PLL from
the wrong base. Every HS mode failed. PWM still worked. The
attribute is persistent. The wrong value survived reboots.
Set 38.4 MHz in the table. Read the attribute first. Write it
only when the value differs or the read fails. Log a changed
value and a failed read. Verified on the Galaxy Book 4 Edge.
Reviewed by: imp (mentor)
Sponsored by: Samsung Electronics
Differential Revision: https://reviews.freebsd.org/D59297
ng_bridge: do not move hosts from learnMac=0 hooks
ng_bridge(4) says the node does not learn MAC addresses on uplink
hooks. However, learnMac was only checked when inserting a new
host. A host already known on a link hook was still moved if a
packet with that source address arrived on an uplink hook.
The nature of this is that inbound unicast to that host then
never arrives (the destination is known on the incoming hook).
Unknown unicast after timeout is still sent only to uplink, so
the host is not re-learned. The interface stays up and outbound
may still work. This can last minutes or weeks until reboot or
NGM_BRIDGE_MOVE_HOST.
Connecting ng_ether(4) lower to an uplink hook is enough: the
host's own transmit can appear on the uplink and the table entry
moves.
Use the same learnMac test for data-path move as for insert.
[5 lines not shown]
git-arc: SC2223 DoS-via-glob hardening
Quote LOCALBASE and ARC_CMD default assignments so a poisoned
value cannot glob into :'s argv.
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D59130
rtwn: add support for the RTL8723BU
Pushed using the RTL8723BU.
Reviewed by: ziaee, avos, adrian
Relnotes: yes
Differential Revision: https://reviews.freebsd.org/D59205
truss: add -t to select which system calls are reported
truss reports every system call a process makes, which for anything
larger than a toy program buries the calls of interest. Add -t, taking
a comma-separated expression naming the system calls to report.
A term is the name of a system call, which may contain the fnmatch(3)
wildcards; a system call number in decimal; or "@group" naming a group
of related system calls. A term prefixed with '!' excludes what that
one term matches rather than including it, and applies to no other
term. An expression whose terms are all negated subtracts from the set
of every system call; any other expression selects from an empty one.
Terms apply in order and the last one to match a system call decides
whether it is reported. Repeating -t appends, so "-t a -t b" and
"-t a,b" are equivalent. An empty term is ignored, so an empty
expression filters nothing and a stray comma is not an error.
truss -t @file,@net fetch https://www.freebsd.org/
truss -t '!@memory' make buildworld
[60 lines not shown]
powerpc/radix: take the pmap lock in mmu_radix_sync_icache()
mmu_radix_sync_icache() walked the page tables with an unlocked
pmap_extract() and passed the result straight to PHYS_TO_DMAP(),
checking only that it was non-zero. Nothing keeps the mapping - or the
page table page holding it - alive across that window: if another thread
of the same process tears a mapping down concurrently, the page table
page can be freed and reused, so pmap_extract() reads arbitrary memory
and returns a bogus physical address. __syncicache() then dereferences
an unmapped direct map address and the kernel takes a data storage
interrupt:
fatal kernel trap:
exception = 0x300 (data storage interrupt)
virtual address = 0xc003317ca6022a00
dsisr = 0x40000000
srr0 = 0xc000000000f59460 (__syncicache)
lr = 0xc000000000f23588 (mmu_radix_sync_icache)
pid = 23878, comm = skyframe-evaluator-
[25 lines not shown]
tpm: Move user copies outside the TPM 1.2 lock
The character-device paths held the transaction and lifecycle lock while
uiomove() accessed user memory. A user page fault could therefore delay
suspend or detach, and a copyout failure occurred while the TPM response
was still active.
Copy commands into the bounded stack buffer before taking the lock. For
reads, validate the response header, buffer the complete response while
the lock is held, finish the TPM transaction, and copy it to userspace
after unlocking. Use a non-blocking allocation so memory pressure
cannot turn response buffering into another lifecycle wait.
NetBSD uses the same separation but limits responses to its fixed 1 KiB
buffer. Allocate the TPM-advertised response length to preserve the
existing FreeBSD support for larger streamed responses.
On a ThinkPad T440p with an STMicro TPM 1.2, a PCR read into a 4 KiB
userspace buffer returned the expected 30-byte response. A deliberately
[7 lines not shown]
tpm: Bound TPM 1.2 locality ownership
A TIS locality must remain active while a command is in flight, but
should be relinquished once the command completes or is abandoned. The
driver retained locality zero after probe, initialization, and resume,
and several transaction error paths returned without releasing it.
Closing the device after writing a command without reading its response
had the same effect.
Track locality ownership and whether a command is awaiting its response.
Release locality after probe, initialization, and resume; retain it only
across a successful command write and its matching response read. Abort
and release on errors, replacement commands, close, and detach.
Wait for locality during ISA probe instead of assuming an immediate
grant, release locality acquired by the probe, and stop treating the
command-style TPM_ACCESS register as restorable state.
On a ThinkPad T440p with an STMicro TPM 1.2, the old driver left
[9 lines not shown]
tpm: Remove Giant from the TPM 1.2 driver
Serialize TPM 1.2 commands, character-device methods, and power
transitions with an sx lock, following the command ownership model used
by the TPM 2.0 driver. Reject new operations once detach starts and
drain the character device before releasing transport resources.
Giant also closed the interrupt race between the final TIS status check
and tsleep. Replace that implicit dependency with a mutex and condition
variable, use an absolute deadline across unrelated wakeups, and make
the interrupt handler MPSAFE.
Create the device node atomically with its softc and finish failed write
transactions so every command path releases its transport state.
The polling path was validated on ThinkPad T430 and T440p systems with
their STMicro TPM 1.2 devices enabled. Exclusive-open behavior, 100
consecutive PCR reads, and module unload and reload completed without
errors on both systems. Two consecutive S3 cycles on each system
[7 lines not shown]
tpm: Restore TPM 1.2 TIS state after resume
Firmware restores the state saved by TPM_ORD_SaveState, but the TIS
interrupt, locality, and command FIFO state are not guaranteed to
survive S3. The legacy driver previously treated resume as a no-op.
Revalidate the interface and device identity, disable and acknowledge
stale interrupts, restore the configured interrupt vector, reacquire
locality zero, and return the FIFO to command-ready state. Also disable
TIS interrupts during initial setup when the device uses polling so
firmware settings cannot leave an unhandled interrupt enabled.
TIS 1.3 Table 22 makes the interrupt control registers locality
protected. Acquire locality before disabling or programming them during
initial setup and resume rather than relying on probe retaining
locality.
Keep TPM self-test outside the resume critical path. It can take
minutes on some TPM 1.2 devices and is not required to restore the
[17 lines not shown]
