host/taint: a pid-less Client still makes its id ambiguous
Verification round on R10-3's own fix. The ambiguity guard detected a duplicate client id by looking it up in the pid map — which is only populated for Clients that carry a sec_pid at all. A pid-less first claimant therefore left no trace, so the next Client claiming the same id looked unique and its pid was used, resolving an ambiguous id: exactly the guess the guard exists to refuse. Reachable, not theoretical — pid-less Clients are ordinary here (the session manager's is one). Reproduced: the bystander app went eligible off a coin-toss owner attribution. Claimed ids are now tracked separately from resolved pids. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -100,13 +100,19 @@ pub struct OwnerCtx {
|
|||||||
impl OwnerCtx {
|
impl OwnerCtx {
|
||||||
pub fn new(snapshot: &GraphSnapshot, pipewire_pulse_pid: Option<u32>) -> Self {
|
pub fn new(snapshot: &GraphSnapshot, pipewire_pulse_pid: Option<u32>) -> Self {
|
||||||
let mut client_pids: BTreeMap<GlobalId, u32> = BTreeMap::new();
|
let mut client_pids: BTreeMap<GlobalId, u32> = BTreeMap::new();
|
||||||
let mut ambiguous: BTreeSet<GlobalId> = BTreeSet::new();
|
// ⚠️ Tracked separately from `client_pids`, and that is the point: a
|
||||||
|
// Client with no `sec_pid` still *claims* its id. Detecting duplicates
|
||||||
|
// by looking in the pid map would let a pid-less first claimant leave
|
||||||
|
// no trace, so the next Client claiming the same id would look unique
|
||||||
|
// and its pid would be used — resolving an ambiguous id, which is the
|
||||||
|
// one guess this guard exists to refuse. Pid-less Clients are ordinary
|
||||||
|
// (the session manager's is one).
|
||||||
|
let mut seen: BTreeSet<GlobalId> = BTreeSet::new();
|
||||||
for client in snapshot.clients() {
|
for client in snapshot.clients() {
|
||||||
if client_pids.contains_key(&client.id) || ambiguous.contains(&client.id) {
|
if !seen.insert(client.id) {
|
||||||
// Two Clients claiming one id: drop it entirely rather than
|
// Two Clients claiming one id: drop it entirely rather than
|
||||||
// pick. See the field docs.
|
// pick. See the field docs.
|
||||||
client_pids.remove(&client.id);
|
client_pids.remove(&client.id);
|
||||||
ambiguous.insert(client.id);
|
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if let Some(pid) = client.sec_pid {
|
if let Some(pid) = client.sec_pid {
|
||||||
|
|||||||
@@ -949,6 +949,48 @@ fn an_ambiguous_client_id_yields_no_fallback_pid() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The ambiguity guard must not depend on the *first* Client claiming an id
|
||||||
|
/// having a `sec_pid`.
|
||||||
|
///
|
||||||
|
/// Found by auditing R10-3 rather than by a failing case: the first cut
|
||||||
|
/// detected a duplicate id by looking it up in the pid map, which is only
|
||||||
|
/// populated for Clients that carry a pid at all. A pid-less Client therefore
|
||||||
|
/// left no trace, and the next Client claiming the same id was treated as
|
||||||
|
/// unique — resolving an ambiguous id, which is exactly the guess the guard
|
||||||
|
/// exists to refuse. Pid-less Clients are ordinary here (`device_node`'s
|
||||||
|
/// session client is one), so this is reachable, not theoretical.
|
||||||
|
#[test]
|
||||||
|
fn a_pidless_first_client_still_makes_its_id_ambiguous() {
|
||||||
|
let mut graph = Graph::new();
|
||||||
|
let hw = graph.device_node("hw-sink", MediaRole::Sink);
|
||||||
|
let call = graph.peerspeak_node("peerspeak", 7);
|
||||||
|
graph.link(call, hw);
|
||||||
|
let sunshine = graph.app_node("sunshine", MediaRole::StreamInput, 3_838);
|
||||||
|
graph.link(hw, sunshine);
|
||||||
|
|
||||||
|
// First claimant has NO sec_pid; second has one.
|
||||||
|
let shared_id = graph.client(None);
|
||||||
|
graph.client_with_id(shared_id, Some(6_011));
|
||||||
|
let app = graph.node(
|
||||||
|
"native-app",
|
||||||
|
MediaRole::StreamOutput,
|
||||||
|
NodeProps {
|
||||||
|
client_id: Some(shared_id),
|
||||||
|
..NodeProps::default()
|
||||||
|
},
|
||||||
|
);
|
||||||
|
graph.link(app, hw);
|
||||||
|
|
||||||
|
assert_partition(
|
||||||
|
&run(&graph, &ctx()),
|
||||||
|
&[],
|
||||||
|
&[
|
||||||
|
("call", call, "peerspeak-owned"),
|
||||||
|
("native-app", app, "unresolved-owner"),
|
||||||
|
],
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
/// The node's own `application.process.id` wins when both are available. It is
|
/// The node's own `application.process.id` wins when both are available. It is
|
||||||
/// a direct statement about the node; the Client's is a one-hop inference, and
|
/// a direct statement about the node; the Client's is a one-hop inference, and
|
||||||
/// they can legitimately differ (a Pulse-emulated node's pid is the app's while
|
/// they can legitimately differ (a Pulse-emulated node's pid is the app's while
|
||||||
|
|||||||
Reference in New Issue
Block a user