feat(cardwired): add a SmartPolicy interface - #155
Merged
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe daemon now exposes Smart Policy D-Bus methods for per-process GPU access. ChangesSmart Policy control
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SmartPolicyInterface
participant EbpfBlocker
participant EbpfMaps
Client->>SmartPolicyInterface: request_process_access(pid, policy, value)
SmartPolicyInterface->>EbpfMaps: validate and update PID policy
EbpfMaps-->>SmartPolicyInterface: return map result
SmartPolicyInterface-->>Client: return D-Bus result
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/cardwire-daemon/src/interface/smart.rs`:
- Around line 37-50: Update the policy handling around the process existence
check and the Allow_dGPU insertion so authorization is bound to the specific
process instance, not just its PID. Capture and store a kernel-verifiable
process identity (such as the process start time) with the map entry, and ensure
enforcement validates that identity before applying policy, preventing stale
entries from affecting PID-reused processes.
- Around line 76-91: Update the map reads in the status-returning method
containing pid_map and forced_map so only MapError::KeyNotFound leaves the
policy unset; propagate every other Aya map error to the D-Bus caller as
fdo::Error::Failed. Replace both if let Ok(...) branches with explicit error
matching while preserving the existing Allowed and Forced status updates for
successful lookups.
- Around line 42-65: Update the policy handling match to replace prior state for
each PID: make “Default” remove the PID from both maps, and have “Allow_dGPU”
remove it from forced_map before inserting into pid_map, while both force
policies remove it from pid_map before inserting into forced_map. Ignore only
MapError::KeyNotFound from removals and propagate all other map errors; preserve
existing insertion error conversion in the policy update method.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4de11abf-3d6a-4264-b946-11535d177721
📒 Files selected for processing (6)
crates/cardwire-daemon/src/analyzer/models.rscrates/cardwire-daemon/src/daemon.rscrates/cardwire-daemon/src/interface/mod.rscrates/cardwire-daemon/src/interface/smart.rscrates/cardwire-daemon/src/models.rscrates/cardwire-ebpf-userspace/src/lib.rs
Comment on lines
+37
to
+50
| // Check if the process exists, leave if it doesnt | ||
| if !Path::new(&format!("/proc/{}", pid)).exists() { | ||
| return Err(fdo::Error::Failed("process doesn't exist".to_string())); | ||
| } | ||
| // Match the policy and add the pid to the corresponding ebpf map | ||
| match policy.as_str() { | ||
| // Default, do nothing | ||
| "Default" => Ok(()), | ||
| // Equivalent to CARDWIRE_ALLOW=1, show both iGPU and dGPU | ||
| "Allow_dGPU" => { | ||
| let mut pid_map = self.pid_map.write().await; | ||
| pid_map | ||
| .insert(pid, 0, 0) | ||
| .map_err(|err| fdo::Error::Failed(err.to_string())) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Bind the policy to a process instance, not only a PID.
By the process-binding spell, /proc/<pid> existence is a TOCTOU check. The process can exit after the check and before the map insertion. If exit cleanup occurs before insertion, the stale entry can apply to a later process that reuses the PID.
Store and validate a process-instance identity during kernel enforcement. Do not treat a PID-only map entry as a stable authorization target.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/cardwire-daemon/src/interface/smart.rs` around lines 37 - 50, Update
the policy handling around the process existence check and the Allow_dGPU
insertion so authorization is bound to the specific process instance, not just
its PID. Capture and store a kernel-verifiable process identity (such as the
process start time) with the map entry, and ensure enforcement validates that
identity before applying policy, preventing stale entries from affecting
PID-reused processes.
Member
Author
There was a problem hiding this comment.
this will be fixed in another PR, make a issue for it
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
repo="OpenGamingCollective/cardwire"
pr_url="https://github.com/OpenGamingCollective/cardwire/pull/155"
comment_url="https://github.com/OpenGamingCollective/cardwire/pull/155#discussion_r3734254717"
body=$(cat <<EOF
## Summary
Bind Smart Policy authorization to a process instance instead of only a PID.
## Rationale
\`SmartPolicyInterface::request_process_access\` checks that \`/proc/<pid>\` exists and then inserts a PID-only entry into the eBPF policy maps. The process can exit between the existence check and the insertion. If the PID is later reused, a stale policy entry can affect the new process.
## Affected areas
- \`crates/cardwire-daemon/src/interface/smart.rs\`
- Smart Policy eBPF maps and their kernel enforcement logic
## Required changes
- Capture a kernel-verifiable process-instance identity when a Smart Policy entry is created. A process start-time identity is one possible mechanism.
- Store this identity with the Smart Policy map entry.
- Validate the stored identity against the current process identity during kernel enforcement.
- Ignore or remove policy entries that do not match the current process instance.
- Preserve the intended behavior for \`Allow_dGPU\`, \`Force_dGPU\`, and GPU-specific policies.
## Acceptance criteria
- A policy created for a process does not apply to a different process that later reuses the same PID.
- Kernel enforcement verifies process-instance identity before it applies a Smart Policy entry.
- The implementation includes tests or another documented validation method for PID reuse or stale-entry handling.
## Backlinks
- Deferred from PR `#155`: ${pr_url}
- Review discussion: ${comment_url}
Requested by: `@luytan`
EOF
)
gh issue create \
--repo "$repo" \
--title "Bind Smart Policy entries to process instances to prevent PID reuse" \
--body "$body" \
--assignee "luytan"Length of output: 2088
@luytan, the wizard has opened and assigned follow-up issue #156: Bind Smart Policy entries to process instances to prevent PID reuse.
It includes the required changes, acceptance criteria, and backlinks to PR #155 and this review discussion.
You are interacting with an AI system.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/cardwire-daemon/src/interface/smart.rs (1)
47-85: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftSerialize each policy transition.
The handler releases the contrary-map write lock before it inserts into the target map. Two concurrent policy requests can interleave their map operations and leave the same PID in both maps.
Use one shared transition lock for all writers, or encode the policy in one eBPF map. The analyzer shares these maps, so include its map mutations in the same consistency design.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/cardwire-daemon/src/interface/smart.rs` around lines 47 - 85, Serialize every policy transition with a shared writer lock covering removal from the contrary map and insertion into the target map, including mutations performed by the analyzer. Update the relevant handler methods around pid_map and forced_map so no writer can observe or create an intermediate state where the same PID exists in both maps; retain the existing error propagation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/cardwire-daemon/src/interface/smart.rs`:
- Around line 47-85: Serialize every policy transition with a shared writer lock
covering removal from the contrary map and insertion into the target map,
including mutations performed by the analyzer. Update the relevant handler
methods around pid_map and forced_map so no writer can observe or create an
intermediate state where the same PID exists in both maps; retain the existing
error propagation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f9538afc-0cd0-4991-a6d5-b2283a50d2f9
📒 Files selected for processing (1)
crates/cardwire-daemon/src/interface/smart.rs
This was referenced Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Includes two methods:
Fixes # (issue)
TODO
Checklist: