asusd: open scsi_generic /dev/sgN for SCSI Aura devices - #257
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe SCSI path discovers and prefers existing ChangesSCSI device handling
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant AuraManager
participant Sysfs
participant DevSg
participant ScsiAura
AuraManager->>Sysfs: Walk ancestor devices
Sysfs-->>AuraManager: Return scsi_generic entries
AuraManager->>DevSg: Check /dev/sgN
DevSg-->>AuraManager: Return generic node or unavailable
AuraManager->>ScsiAura: Initialize with /dev/sgN or block node
ScsiAura->>ScsiAura: Execute SCSI tasks
ScsiAura-->>ScsiAura: Log task errors and continue
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@asusd/src/aura_manager.rs`:
- Around line 317-335: Update the SCSI device-event handling around the
block-node initialization and monitor matching so a later scsi_generic
(/dev/sgN) add event triggers initialization retry for the same device. Replace
the retained /dev/sdX handle or re-run the relevant DeviceHandle initialization
once sg_node_for_block resolves, while preserving existing behavior when the
block event arrives after the generic node.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bfefcb49-ea96-4492-aa70-c133dd53aeb2
📒 Files selected for processing (2)
asusd/src/aura_manager.rsasusd/src/aura_scsi/mod.rs
📜 Review details
🔇 Additional comments (1)
asusd/src/aura_scsi/mod.rs (1)
30-35: LGTM!
SG_IO with a vendor CDB on the block node (/dev/sdX) requires CAP_SYS_RAWIO. The hardened asusd systemd unit strips all capabilities (empty CapabilityBoundingSet, NoNewPrivileges=true), so every SG_IO on the block node returned EPERM. write_effect() was doing perform(task).ok(), so the error was silently swallowed; asusd reported success while zero SCSI traffic reached the enclosure (confirmed via usbmon). Resolve the scsi_generic node (/dev/sgN) backing the block device and open that instead. The sg driver gates access at open() through file permissions, so SG_IO works without any capability, the same path sg3_utils and OpenRGB use. On hotplug the sg node can appear just after the block node, so init retries briefly before falling back to the block device. Also stop discarding the ioctl error in write_effect so a future failure can't go invisible again. Verified on a ROG STRIX Arion (0b05:1932): 16/16 SG_IO return 0, the ENE vendor CDBs (mode 0x8021, colour registers 0x8160+, apply 0x80a0) appear in usbmon, and the LEDs change.
538ed80 to
333feea
Compare
|
BTW, we own sg.rs too if you want to take a look into it too. The one on crates.io is not supported anymore, like the one made by luke back in 2025, so I took the liberty to pick his version up and import it directly into code. |
|
Nice, took a look. sg.rs sidesteps the cmd_len trap I hit in the kernel driver: it sets cmd_len from the slice length, so the 16-byte vendor CDB works as-is. The kernel sizes the CDB by opcode, 0xec maps to 10, cdb[13] (the ENE data-length byte) drops silently, and I had to hand-build the request there to force 16. |
SCSI Aura devices (e.g. ROG STRIX Arion,
0b05:1932) showed up on D-Bus andasusctl scsireported success, but no traffic ever reached the enclosure. The LEDs never changed.write_effect()opened the block node (/dev/sdX) and issuedSG_IOwith the vendor CDB. SG_IO with a vendor command on a block node requiresCAP_SYS_RAWIO, but the hardened asusd unit strips every capability (emptyCapabilityBoundingSet,NoNewPrivileges=true), so every ioctl returnedEPERM.write_effect()then didperform(task).ok()and threw the error away, so asusd reported success while zero commands hit the wire. strace showedioctl(N, SG_IO, ...) = -1 EPERMsixteen times per write, and usbmon showed only baseline usb-storage traffic.Fix: resolve the
scsi_genericnode (/dev/sgN) backing the block device and open that instead. The sg driver gates access atopen()through the file permissions, so SG_IO works with no capabilities, the same path sg3_utils and OpenRGB use. I also stopped swallowing the ioctl error inwrite_effectso this can't go silent again.Verified on an Arion: 16/16 SG_IO return 0, the ENE vendor CDBs (mode
0x8021, colour registers0x8160+, apply0x80a0) show up in usbmon, and the LEDs change.Two files, no kernel changes. This is the working userspace reference for the SCSI device-handler driver we discussed.
On hotplug the sg node can lag the block node by a moment, so init retries briefly before falling back to the block device; at startup the node already exists, so the first attempt hits with no delay.