Skip to content

udev,login: update ACL on static device nodes#39071

Merged
poettering merged 2 commits intosystemd:mainfrom
yuwata:udev-login-update-acl-on-static-device-nodes
Sep 23, 2025
Merged

udev,login: update ACL on static device nodes#39071
poettering merged 2 commits intosystemd:mainfrom
yuwata:udev-login-update-acl-on-static-device-nodes

Conversation

@yuwata
Copy link
Member

@yuwata yuwata commented Sep 22, 2025

Fixes regression caused by #36444.
Fixes #39043.

@yuwata yuwata force-pushed the udev-login-update-acl-on-static-device-nodes branch from df6aaa7 to ccb0a48 Compare September 22, 2025 11:32
@yuwata yuwata marked this pull request as ready for review September 22, 2025 14:51
@github-actions github-actions bot added the please-review PR is ready for (re-)review by a maintainer label Sep 22, 2025
@yuwata yuwata force-pushed the udev-login-update-acl-on-static-device-nodes branch from ccb0a48 to aad030a Compare September 23, 2025 01:24
if (acl_calc_mask(&acl) < 0)
return -errno;

if (acl_set_file(FORMAT_PROC_FD_PATH(fd), ACL_TYPE_ACCESS, acl) < 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, i wonder if acl_set_fd() works these days?

(but maybe let's do that independently, in particular to make this simple to break point)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(the reason we use acl_set_file() here was either some O_PATH messiness around libacl, or because xattr syscalls didn't do fd-based stuff originally, i don't remember. but i think both are nowadays solved in libacl, but dunno)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, will check and update later in another PR.

@poettering poettering added good-to-merge/with-minor-suggestions and removed please-review PR is ready for (re-)review by a maintainer labels Sep 23, 2025
This effectively reverts 1abb592.
No functional change, preparation for the next commit.
In the commit c960ca2, the logic of
updating ACL on device node was moved from logind to udevd, but at that
time, mistakenly removed the logic for static nodes.

Fixes a regression caused by c960ca2 (v258).
Fixes systemd#39043.
@yuwata yuwata force-pushed the udev-login-update-acl-on-static-device-nodes branch from aad030a to 2c762d9 Compare September 23, 2025 10:58
@yuwata yuwata added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed good-to-merge/with-minor-suggestions labels Sep 23, 2025
@poettering poettering merged commit daf99b0 into systemd:main Sep 23, 2025
50 of 56 checks passed
@github-actions github-actions bot removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Sep 23, 2025
@yuwata yuwata deleted the udev-login-update-acl-on-static-device-nodes branch September 23, 2025 12:51
@keszybz
Copy link
Member

keszybz commented Oct 13, 2025

Backport queued for v258.1

github-merge-queue bot pushed a commit to NixOS/nixpkgs that referenced this pull request Oct 20, 2025
This PR systemd/systemd#36444 caused this bug
systemd/systemd#39043, which is fixed in
this PR systemd/systemd#39071. In short,
`uaccess` doesn't work with `OPTIONS+="static_node=..."` udev rules,
and `/dev/snd/timer` is a static node. 258.1 needs to wait for the
next staging cycle, so for now let's just use a non-static node.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Steaminput no longer works after update to 258-1.1

3 participants