Improve path readability check#176
Conversation
sjoerdsimons
requested review from
bill-shuzhou-liu,
charis-poag-amd,
dmitrii-galantsev,
marifamd and
oliveiradan
as code owners
sjoerdsimons
force-pushed
the
improve-readability-test
branch
2 times, most recently
from
883ba29 to
8d0aa28
Compare
sjoerdsimons
changed the title
There was a problem hiding this comment.
Pull request overview
This PR attempts to fix false positives in device readability checks on modern systems that use udev uaccess functionality with ACLs. Instead of only checking traditional Unix user/group permissions, the code now attempts to open the device file to test actual access.
Changes:
- Added ACL-aware access check by attempting to open the device file before falling back to traditional permission checks
- Removed trailing whitespace from line 1306
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
On modern system e.g. render nodes are made accessible via the udev uaccess functionality, which adds the logged in user to the ACL of the device. This means just checking for user and group is bound to give false positives. Instead use os.access as a first check Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
sjoerdsimons
force-pushed
the
improve-readability-test
branch
from
8acceea to
562ad24
Compare
|
This is actually the wrong repo because ROCm has switched to mono-repo. Could you re-open it here: https://github.com/ROCm/rocm-systems/tree/develop/projects/amdsmi |
Will do! would be great if there was a note in this repo to make that more discoverable for people ;) |
|
It's really confusing but there are still releases from the "legacy" train coming here. Basically pure cherry picks for the legacy branch. |
2 participants
On modern system e.g. render nodes are made accessible via the udev uaccess functionality, which adds the logged in user to the ACL of the device. This means just checking for user and group is bound to give false positives. Instead simply try to open the device as a test and only if that fails try to determine the cause based on user/groups
Motivation
Avoid false positives in readability tets
Technical Details
On modern system e.g. render nodes are made accessible via the udev uaccess functionality, which adds the logged in user to the ACL of the device. This means just checking for user and group is bound to give false positives. Instead simply try to open the device as a test and only if that fails try to determine the cause based on user/groups
Test Plan
Tested on framework desktop with devices managed by uaccess
Test Result
Permission denied warnings without this change; No warnings without
Submission Checklist