host/observer: close the snd_aloop absent-driver leak (Codex re-review)
Codex's re-review of the phase-3 fixes confirmed finding 1/5/6 closed but found the finding-2 fix incomplete: the denylist only rejected a *present* snd_aloop driver, so an snd_aloop node whose alsa.driver_name was not copied onto the node still classified session_device=true — the original leak. The absence is reachable: PipeWire >=1.2.6 stopped overwriting node props with card props, and WirePlumber only began copying alsa.* onto nodes in 0.5.13. Fix: session_device now requires a PRESENT, non-denied ALSA driver; a missing alsa.driver_name fails closed to NotSessionDevice (a real card without the prop is over-excluded — safe; recovering it needs reading the driver from the backing Device global, owed to a later round). Mutation-verified: reverting to fail-open on absence is killed by classify_alsa_without_driver_name_fails_closed. Also: corrected the finding-3 limitation doc to cite PipeWire's object.serial identity contract rather than overclaiming the live gate proves it (Codex P3, non-blocking). 121 unit + live gate row 6 green, clippy clean, observer files fmt-clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -61,9 +61,10 @@ const HARDWARE_PCM_FACTORIES: &[&str] = &[
|
|||||||
/// would let tainted audio loop back untainted (Codex phase-3 review,
|
/// would let tainted audio loop back untainted (Codex phase-3 review,
|
||||||
/// finding 2). `factory.name` alone cannot distinguish these from a real
|
/// finding 2). `factory.name` alone cannot distinguish these from a real
|
||||||
/// card — `snd_aloop` presents as `api.alsa.pcm.{sink,source}` exactly like
|
/// card — `snd_aloop` presents as `api.alsa.pcm.{sink,source}` exactly like
|
||||||
/// `snd_hda_intel` — so this is a necessary denylist layered under the
|
/// `snd_hda_intel` — so a real ALSA terminal must present an `alsa.driver_name`
|
||||||
/// factory allowlist, keyed on the one property that does distinguish them
|
/// that is **present and not on this denylist**; a missing driver fails closed
|
||||||
/// (`alsa.driver_name`, measured present on real ALSA nodes).
|
/// (see [`classify`]). `snd_dummy` is intentionally absent: it is virtual but
|
||||||
|
/// does not couple playback to capture, so it is not a loopback hazard.
|
||||||
const NON_TERMINAL_ALSA_DRIVERS: &[&str] = &["snd_aloop"];
|
const NON_TERMINAL_ALSA_DRIVERS: &[&str] = &["snd_aloop"];
|
||||||
|
|
||||||
/// The three node properties the classifier reads, exactly as the adapter
|
/// The three node properties the classifier reads, exactly as the adapter
|
||||||
@@ -88,8 +89,11 @@ pub struct DeviceClaim {
|
|||||||
/// `alsa.driver_name` — the kernel driver behind an ALSA node (e.g.
|
/// `alsa.driver_name` — the kernel driver behind an ALSA node (e.g.
|
||||||
/// `snd_hda_intel`, `snd_usb_audio`, `snd_aloop`). Needed because the
|
/// `snd_hda_intel`, `snd_usb_audio`, `snd_aloop`). Needed because the
|
||||||
/// factory allowlist cannot tell a real card from a loopback driver that
|
/// factory allowlist cannot tell a real card from a loopback driver that
|
||||||
/// shares the same factory; a driver on [`NON_TERMINAL_ALSA_DRIVERS`]
|
/// shares the same factory. `session_device` requires this to be
|
||||||
/// forfeits `session_device`. Absent on non-ALSA backends.
|
/// **present and not** on [`NON_TERMINAL_ALSA_DRIVERS`]; a driver on the
|
||||||
|
/// denylist, or an absent value, both fail closed (see [`classify`]).
|
||||||
|
/// May be absent on non-ALSA backends or on version pairings that do not
|
||||||
|
/// copy `alsa.*` onto the node.
|
||||||
pub alsa_driver_name: Option<String>,
|
pub alsa_driver_name: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -132,12 +136,21 @@ pub fn classify(claim: &DeviceClaim, device_resolved: bool) -> Classification {
|
|||||||
.factory_name
|
.factory_name
|
||||||
.as_deref()
|
.as_deref()
|
||||||
.is_some_and(|f| HARDWARE_PCM_FACTORIES.contains(&f));
|
.is_some_and(|f| HARDWARE_PCM_FACTORIES.contains(&f));
|
||||||
let non_terminal_driver = claim
|
// A **present, non-denied** ALSA driver is required — absence fails closed
|
||||||
|
// (Codex phase-3 re-review). `alsa.driver_name` is not copied onto the
|
||||||
|
// node on every PipeWire/WirePlumber version pairing (PipeWire ≥1.2.6
|
||||||
|
// stopped overwriting node props with card props; WirePlumber only began
|
||||||
|
// copying `alsa.*` onto nodes in 0.5.13), so a *missing* value must not be
|
||||||
|
// read as "not a loopback" — that is exactly the hole an `snd_aloop` node
|
||||||
|
// without the property would slip through. A real card whose node lacks
|
||||||
|
// the driver is instead over-excluded (keeps its owner keys — safe);
|
||||||
|
// recovering `session_device` for it needs reading the driver from the
|
||||||
|
// backing Device global, which is owed to a later round.
|
||||||
|
let driver_ok = claim
|
||||||
.alsa_driver_name
|
.alsa_driver_name
|
||||||
.as_deref()
|
.as_deref()
|
||||||
.is_some_and(|d| NON_TERMINAL_ALSA_DRIVERS.contains(&d));
|
.is_some_and(|d| !NON_TERMINAL_ALSA_DRIVERS.contains(&d));
|
||||||
let is_hardware_pcm =
|
let is_hardware_pcm = claim.device_api.is_some() && on_factory_allowlist && driver_ok;
|
||||||
claim.device_api.is_some() && on_factory_allowlist && !non_terminal_driver;
|
|
||||||
if is_hardware_pcm {
|
if is_hardware_pcm {
|
||||||
Classification::SessionDevice
|
Classification::SessionDevice
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -31,10 +31,13 @@
|
|||||||
//! The adapter drops such a global before it reaches [`RegistryModel`], so
|
//! The adapter drops such a global before it reaches [`RegistryModel`], so
|
||||||
//! readiness can reach `Complete` while permanently omitting that Link — an
|
//! readiness can reach `Complete` while permanently omitting that Link — an
|
||||||
//! invisible edge that could hide tainted ancestry. **Not reachable in
|
//! invisible edge that could hide tainted ancestry. **Not reachable in
|
||||||
//! practice:** every real Link global carries `object.serial` (confirmed by
|
//! practice:** PipeWire's native protocol defines `object.serial` as the
|
||||||
//! the live gate, which only counts links the strict parser admits). A full
|
//! unique identity every global carries, so a Link without one requires a
|
||||||
//! fix needs a pure "required-observation-failed" token that holds readiness
|
//! protocol/server failure, not ordinary churn. (The live gate is
|
||||||
//! false; deferred rather than built for a case that does not occur.
|
//! consistent with this but does not *prove* it — it only counts Links the
|
||||||
|
//! strict parser already admitted.) A full fix needs a pure
|
||||||
|
//! "required-observation-failed" token that holds readiness false; deferred
|
||||||
|
//! rather than built for a case that does not occur.
|
||||||
//! - *Removal generation ordering assumes no removal is silently lost.* On a
|
//! - *Removal generation ordering assumes no removal is silently lost.* On a
|
||||||
//! recycled id with two live claimants, [`Self::on_removed`] retires the
|
//! recycled id with two live claimants, [`Self::on_removed`] retires the
|
||||||
//! oldest generation first; if the *first* generation's removal was never
|
//! oldest generation first; if the *first* generation's removal was never
|
||||||
|
|||||||
@@ -142,6 +142,26 @@ fn classify_invented_bluez_factories_are_not_session_devices() {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn classify_alsa_without_driver_name_fails_closed() {
|
||||||
|
// Codex re-review: a missing `alsa.driver_name` must NOT grant
|
||||||
|
// session_device — an snd_aloop node whose driver prop was not copied onto
|
||||||
|
// the node would otherwise slip through. Absence fails closed.
|
||||||
|
for factory in ["api.alsa.pcm.sink", "api.alsa.pcm.source"] {
|
||||||
|
let claim = DeviceClaim {
|
||||||
|
device_id: Some(gid(7)),
|
||||||
|
device_api: Some("alsa".to_string()),
|
||||||
|
factory_name: Some(factory.to_string()),
|
||||||
|
alsa_driver_name: None,
|
||||||
|
};
|
||||||
|
assert_eq!(
|
||||||
|
classify(&claim, true),
|
||||||
|
Classification::NotSessionDevice,
|
||||||
|
"absent driver on {factory} must fail closed"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn classify_snd_aloop_is_not_a_session_device() {
|
fn classify_snd_aloop_is_not_a_session_device() {
|
||||||
// Finding 2: an ALSA loopback presents with an allowlisted factory and
|
// Finding 2: an ALSA loopback presents with an allowlisted factory and
|
||||||
|
|||||||
Reference in New Issue
Block a user