G10: keep the DRM nodes at minor_perm across boots (logindevperm, drm-perms service) #12

Merged

toasterson merged 11 commits from g10-perms into main 2026-10-05 16:47:12 +00:00

Conversation 0 Commits 11 Files changed 9 +1636 -16

toasterson commented 2026-10-03 19:56:37 +00:00

Owner

Copy link

Anima work item: G10 01a102fe-2d96-7450-b51e-47446180ba24

Stacked on #10 (g7-coexist).

Root cause

The packaged virtio_gpu and cdrmnull nodes were 0600 root root after a boot although /etc/minor_perm said 0666 root sys. minor_perm was right. The cause is the illumos-gate /etc/logindevperm rule

/dev/vt/console_user	0600	/dev/dri/*		# dri devices

/dev/vt/console_user points to /dev/console while no VT is active. The ttymon of console-login:default (-d /dev/console) opens the console at every boot and after every console logout, then calls di_devperm_logout(). That function chowns every /dev/dri node to root's passwd owner and group (root root) and acl_strips it to 0600.

devfs keeps the change in a shadow node on the on-disk /devices. dv_shadow_node prefers the shadow to minor_perm, and devfs_reset_perm (what update_drv -m triggers) skips shadowed nodes. Only rem_drv -C removes the shadow; IPS removes a driver with rem_drv without -C. So the wrong values survive reboots, reloads and reinstalls. This explains both the G7 findings and R0's note about drivers registered at boot.

Evidence from the first boot after a fresh package install, traced with anonymous DTrace on devfs_setattr/devfs_setsecattr (logs/hephaistos/illumos-drm/20261003-g10-run1-coldboot-repro.log in the workspace):

• The only writer was ttymon pid 353, at 18:31:51.56, 1.5 s after device/local. It made 3 changes to each of the 4 nodes.
• Every change has the stack ttymon_express → open_device → di_devperm_logout → logindevperm → dir_dev_acc → setdevaccess.
• A ZFS snapshot shows the shadow nodes crw------- root root (…-run1-shadow.log).

Fix

drmadm perms is run by the new transient service svc:/system/graphics/drm-perms (packaging/drm-perms.xml, in driver/graphics/drmadm). The service runs at every boot after device/local and before every console-login instance (an optional_all dependent, so a failure cannot block the console). It also runs when the package is installed (restart_fmri of manifest-import). It does two things:

1. Restricts the logindevperm rule. Each rule for /dev/dri without a driver list gets the legacy drivers' list (driver=i915,radeon), the documented logindevperm(5) mechanism.
   • The legacy stack keeps the console-user assignment unchanged; the new stack's nodes keep minor_perm.
   • The file is parsed as libdevinfo parses it. It is replaced by rename(2), keeping its owner and mode, and the rewrite is idempotent.
   • A rewritten rule longer than libdevinfo's 254-byte line is refused, and the file is left unchanged.
   • Pattern rules and admin rules are left as written and reported.
2. Resets clobbered nodes. New-stack nodes that differ from their minor_perm entry are reset to it. The entry is chosen the way the kernel's add_minorperm/dev_minorperm choose it. Only links whose driver is already loaded are followed: a lookup would attach the driver, and a loaded DRM module makes drmadm select refuse.

drmadm status shows the rules and the nodes. The R0 security model is unchanged: render nodes are open to all, and card nodes are 0666 with master semantics in the core. README has a new Packaging section with the rule, the reasons, the rejected alternatives (per-minor perms, a post-console-login chmod, PRIVONLY_DEV minors) and the consequences:

• /etc/logindevperm is a preserve=true file of SUNWcs. A later SUNWcs copy of it will not be installed while the edit is present; pkg revert it and drmadm perms restricts it again.
• The restriction stays after the drmadm package is removed.

Tests

tools/test/run-g10-test.sh phases:

• arm: anonymous DTrace for the next boot; REVERT=1 restores the packaged rule first.
• coldboot:
  • drm-perms came online in this boot before console-login.
  • From the trace: ttymon changed no DRM node, and drm-perms' rename came before ttymon read the file.
  • Every new-stack node is 0666 root sys, and nobody opens every card and render node read-write.
  • A console-login restart, which is what a logout does, changes nothing.
  • drmadm status lists each node.
• rewrite, minorperm, repair, noload, driver-filter, upgrade.

run-g7-test.sh install now packages drm-perms.xml.

Results on fresh guests of golden-20260930 on hephaistos (2 vCPUs, 3 GiB, virtio-gpu-gl). Logs are logs/hephaistos/illumos-drm/20261003-g10-* in the workspace; the first line of each gives the revision.

• Before the fix (run1, G7 packages): coldboot fails 19 checks — the 4 nodes 0600 root root with their summary after the boot and again after a console-login restart (10), 4 opens by nobody, ttymon changing a node at the restart, the rule applying to every driver, and the 3 checks of the then-missing service.

• Full sequence (run3; drmadm at 1284ac0; the only later drmadm change is the round-2 path_to_inst fix, which run4 re-ran). Every phase passes:

  Phase	Checks
  traced boot with the packaged rule (drm-perms' rename 1.3 s before ttymon's read; no ttymon change to a DRM node)	29
  noload	2
  minorperm	3
  rewrite	17
  driver-filter	4
  repair (fault reproduced, then repaired)	17
  upgrade	15
  coldboot after select new	25
  coldboot after select legacy	8
  G7 select-new	13
  G7 after-new	7
  G7 select-legacy	4
  G7 after-legacy	12
  G7 i915n	2
  G7 select-root	3
  G7 rollback	5
  run-r5a-test.sh (R0–R5a, packaged drivers removed, LEGACY_GUARD=skip)	75, 0 FAIL

• Final revision (run4, fresh guest at 5600545; later commits are README-only): first boot after the installation coldboot 25, traced boot with the packaged rule 29 (drm-perms' rename 2.1 s before ttymon's read; no ttymon change), noload 2, G7 select-new 13, repair 17, all PASS (…-g10-run4-*).

• Also: the first boot of a fresh install into a new boot environment (run2 at 5eb9710) passed coldboot (25). The drm-perms log shows the rule restricted in that boot before console-login.

• Regression found and fixed on the way: run2's G7 select-new showed that the first drmadm status attached the drivers by following the links (fixed in 0db4a73). run3's first coldboot found that pseudo devices are not in path_to_inst (fixed in 1284ac0).

Review

Two Opus review rounds against the Anima bar. Round 1 rejected, and every point was fixed:

• the minor_perm entry order (now the kernel's);
• the cold-boot ordering not proven (now the anonymous boot trace);
• a vacuous status check;
• libdevinfo's 255-byte line limit;
• pattern rules;
• the preserve=true trade-off in the docs;
• temp-file cleanup and the concurrency argument (now tested with 20 parallel runs);
• admin rules naming a new-stack driver.

Round 2 approved. Its two non-blocking items are fixed in 5ab77fe:

• several drivers listed for one path in path_to_inst (the loaded one is taken; more than one loaded is reported);
• coldboot now fails when the boot that arm armed has no trace.

Open issues / notes

• The driver of a /dev/dri link comes from /etc/path_to_inst. Once i915n ships, an Intel GPU path will list i915 and i915n. drmadm takes the loaded one, but this has only been reasoned through, not tested with a real second driver (G5).

• libdevinfo's check_driver_match() reads the driver name after di_fini(), a use-after-free in illumos-gate. The legacy rule's driver list now depends on it. It works today (driver-filter); this is noted in the README and not filed.

• The rule itself is illumos-gate's. Restricting it upstream would make the edit unnecessary; that is the owner's call.

Anima work item: G10 `01a102fe-2d96-7450-b51e-47446180ba24` Stacked on #10 (`g7-coexist`). ## Root cause The packaged `virtio_gpu` and `cdrmnull` nodes were `0600 root root` after a boot although `/etc/minor_perm` said `0666 root sys`. `minor_perm` was right. The cause is the illumos-gate `/etc/logindevperm` rule ``` /dev/vt/console_user 0600 /dev/dri/* # dri devices ``` `/dev/vt/console_user` points to `/dev/console` while no VT is active. The ttymon of `console-login:default` (`-d /dev/console`) opens the console at every boot and after every console logout, then calls `di_devperm_logout()`. That function chowns every `/dev/dri` node to root's passwd owner and group (`root root`) and `acl_strip`s it to 0600. devfs keeps the change in a shadow node on the on-disk `/devices`. `dv_shadow_node` prefers the shadow to `minor_perm`, and `devfs_reset_perm` (what `update_drv -m` triggers) skips shadowed nodes. Only `rem_drv -C` removes the shadow; IPS removes a driver with `rem_drv` without `-C`. So the wrong values survive reboots, reloads and reinstalls. This explains both the G7 findings and R0's note about drivers registered at boot. Evidence from the first boot after a fresh package install, traced with anonymous DTrace on `devfs_setattr`/`devfs_setsecattr` (`logs/hephaistos/illumos-drm/20261003-g10-run1-coldboot-repro.log` in the workspace): - The only writer was `ttymon` pid 353, at 18:31:51.56, 1.5 s after `device/local`. It made 3 changes to each of the 4 nodes. - Every change has the stack `ttymon_express → open_device → di_devperm_logout → logindevperm → dir_dev_acc → setdevaccess`. - A ZFS snapshot shows the shadow nodes `crw------- root root` (`…-run1-shadow.log`). ## Fix `drmadm perms` is run by the new transient service `svc:/system/graphics/drm-perms` (`packaging/drm-perms.xml`, in `driver/graphics/drmadm`). The service runs at every boot after `device/local` and before every `console-login` instance (an `optional_all` dependent, so a failure cannot block the console). It also runs when the package is installed (`restart_fmri` of `manifest-import`). It does two things: 1. **Restricts the logindevperm rule.** Each rule for `/dev/dri` without a driver list gets the legacy drivers' list (`driver=i915,radeon`), the documented `logindevperm(5)` mechanism. - The legacy stack keeps the console-user assignment unchanged; the new stack's nodes keep `minor_perm`. - The file is parsed as libdevinfo parses it. It is replaced by `rename(2)`, keeping its owner and mode, and the rewrite is idempotent. - A rewritten rule longer than libdevinfo's 254-byte line is refused, and the file is left unchanged. - Pattern rules and admin rules are left as written and reported. 2. **Resets clobbered nodes.** New-stack nodes that differ from their `minor_perm` entry are reset to it. The entry is chosen the way the kernel's `add_minorperm`/`dev_minorperm` choose it. Only links whose driver is already loaded are followed: a lookup would attach the driver, and a loaded DRM module makes `drmadm select` refuse. `drmadm status` shows the rules and the nodes. The R0 security model is unchanged: render nodes are open to all, and card nodes are 0666 with master semantics in the core. README has a new **Packaging** section with the rule, the reasons, the rejected alternatives (per-minor perms, a post-console-login chmod, `PRIVONLY_DEV` minors) and the consequences: - `/etc/logindevperm` is a `preserve=true` file of SUNWcs. A later SUNWcs copy of it will not be installed while the edit is present; `pkg revert` it and `drmadm perms` restricts it again. - The restriction stays after the drmadm package is removed. ## Tests `tools/test/run-g10-test.sh` phases: - `arm`: anonymous DTrace for the next boot; `REVERT=1` restores the packaged rule first. - `coldboot`: - drm-perms came online in this boot before console-login. - From the trace: ttymon changed no DRM node, and drm-perms' rename came before ttymon read the file. - Every new-stack node is `0666 root sys`, and `nobody` opens every card and render node read-write. - A console-login restart, which is what a logout does, changes nothing. - `drmadm status` lists each node. - `rewrite`, `minorperm`, `repair`, `noload`, `driver-filter`, `upgrade`. `run-g7-test.sh install` now packages `drm-perms.xml`. Results on fresh guests of `golden-20260930` on hephaistos (2 vCPUs, 3 GiB, virtio-gpu-gl). Logs are `logs/hephaistos/illumos-drm/20261003-g10-*` in the workspace; the first line of each gives the revision. - **Before the fix** (run1, G7 packages): `coldboot` fails 19 checks — the 4 nodes `0600 root root` with their summary after the boot and again after a console-login restart (10), 4 opens by `nobody`, ttymon changing a node at the restart, the rule applying to every driver, and the 3 checks of the then-missing service. - **Full sequence** (run3; drmadm at `1284ac0`; the only later drmadm change is the round-2 `path_to_inst` fix, which run4 re-ran). Every phase passes: | Phase | Checks | | --- | --- | | traced boot with the packaged rule (drm-perms' rename 1.3 s before ttymon's read; no ttymon change to a DRM node) | 29 | | `noload` | 2 | | `minorperm` | 3 | | `rewrite` | 17 | | `driver-filter` | 4 | | `repair` (fault reproduced, then repaired) | 17 | | `upgrade` | 15 | | coldboot after `select new` | 25 | | coldboot after `select legacy` | 8 | | G7 `select-new` | 13 | | G7 `after-new` | 7 | | G7 `select-legacy` | 4 | | G7 `after-legacy` | 12 | | G7 `i915n` | 2 | | G7 `select-root` | 3 | | G7 `rollback` | 5 | | `run-r5a-test.sh` (R0–R5a, packaged drivers removed, `LEGACY_GUARD=skip`) | 75, 0 FAIL | - **Final revision** (run4, fresh guest at `5600545`; later commits are README-only): first boot after the installation `coldboot` 25, traced boot with the packaged rule 29 (drm-perms' rename 2.1 s before ttymon's read; no ttymon change), `noload` 2, G7 `select-new` 13, `repair` 17, all PASS (`…-g10-run4-*`). - **Also:** the first boot of a fresh install into a new boot environment (run2 at `5eb9710`) passed `coldboot` (25). The drm-perms log shows the rule restricted in that boot before console-login. - **Regression found and fixed on the way:** run2's G7 `select-new` showed that the first `drmadm status` attached the drivers by following the links (fixed in `0db4a73`). run3's first coldboot found that pseudo devices are not in `path_to_inst` (fixed in `1284ac0`). ## Review Two Opus review rounds against the Anima bar. Round 1 rejected, and every point was fixed: - the `minor_perm` entry order (now the kernel's); - the cold-boot ordering not proven (now the anonymous boot trace); - a vacuous status check; - libdevinfo's 255-byte line limit; - pattern rules; - the `preserve=true` trade-off in the docs; - temp-file cleanup and the concurrency argument (now tested with 20 parallel runs); - admin rules naming a new-stack driver. Round 2 approved. Its two non-blocking items are fixed in `5ab77fe`: - several drivers listed for one path in `path_to_inst` (the loaded one is taken; more than one loaded is reported); - `coldboot` now fails when the boot that `arm` armed has no trace. ## Open issues / notes - The driver of a `/dev/dri` link comes from `/etc/path_to_inst`. Once `i915n` ships, an Intel GPU path will list `i915` and `i915n`. `drmadm` takes the loaded one, but this has only been reasoned through, not tested with a real second driver (G5). - libdevinfo's `check_driver_match()` reads the driver name after `di_fini()`, a use-after-free in illumos-gate. The legacy rule's driver list now depends on it. It works today (`driver-filter`); this is noted in the README and not filed. - The rule itself is illumos-gate's. Restricting it upstream would make the edit unnecessary; that is the owner's call.

toasterson added 11 commits 2026-10-03 19:56:39 +00:00

drmadm perms, svc:/system/graphics/drm-perms: keep the DRM nodes at minor_perm ... 49e0c0b5f7

The packaged virtio_gpu and cdrmnull nodes were 0600 root root after a
boot although /etc/minor_perm says 0666 root sys. ttymon of
console-login opens /dev/console (/dev/vt/console_user) at every boot
and console logout and runs di_devperm_logout(), which applies the
/etc/logindevperm rule "/dev/dri/*": chown root:root and mode 0600 for
every /dev/dri node. devfs keeps that in the node's shadow on the root
file system, which wins over minor_perm across reboots, reloads and
rem_drv without -C.

drmadm perms restricts each /etc/logindevperm rule for /dev/dri without
a driver list to the legacy drivers (driver=i915,radeon, logindevperm(5)),
and resets new-stack nodes that differ from their minor_perm entry. The
service svc:/system/graphics/drm-perms in driver/graphics/drmadm runs it
at every boot after device/local and before console-login, and when the
package is installed. drmadm status shows the rules and the nodes.
README: a Packaging section with the rule and why.

run-g10-test.sh: the DRM nodes after a cold boot; run-g7-test.sh packages drm-perms.xml ... 5eb97108f2

coldboot: drm-perms ran before console-login, the logindevperm rules
leave the new stack out, every new-stack /dev/dri node is 0666 root sys,
nobody opens each card and render node read-write, and a console-login
restart (a console logout) changes no node (DTrace on ttymon).
driver-filter: a rule restricted to cdrmnull still reaches cdrmnull's
nodes and leaves virtio_gpu's alone. upgrade: the fixed drmadm package
repairs clobbered nodes without a reboot.

drmadm perms: review round 1; run-g10-test.sh: boot trace, rewrite, minorperm and repair phases ... f0f4f8b593

drmadm perms now picks the minor_perm entry the kernel applies (later of
two matching patterns, as add_minorperm() builds the list), moves the
/dev/dri devices of a rule to a rule of their own after a comment line
and refuses a rule longer than libdevinfo's 254-byte line, reports rules
that reach /dev/dri through a pattern, leaves the nodes of drivers an
admin rule names alone, and removes its temporary file on a signal.

run-g10-test.sh: arm (anonymous DTrace for the next boot, REVERT=1 for
the packaged rule) and coldboot's trace check (ttymon changed no DRM
node; drm-perms' rename before ttymon's read), a guard against a
restarted console-login, a per-node check of drmadm status, rewrite
(scratch roots, 20 concurrent runs, line-length refusal), minorperm
(against the kernel) and repair (the fault reproduced, then repaired).
README, drmadm(8): the same, and the preserve=true trade-off.

run-g10-test.sh arm: write the D program with printf (ksh93's echo expands \n) f9afd2e6fc

drmadm: look up only the /dev/dri nodes of loaded drivers; run-g10-test.sh: noload phase ... 0db4a73f2c

drmadm status followed every /dev/dri link, and the lookup in /devices
attached the drivers, so a drmadm select after it refused (G7's
select-new phase on the G10 guest). The driver of a link now comes from
/etc/path_to_inst, and the link is followed only if the driver is loaded.

drmadm: a pseudo device's driver is its node name (no path_to_inst entry) 1284ac0d24

run-g10-test.sh: an empty EXPECT expects no new-stack device 7243649703

README: G10 results 445b7d0448

drmadm: of the drivers path_to_inst lists for a node, the loaded one; run-g10-test.sh: a boot arm armed needs its trace ... 5ab77fee86

Review round 2 (non-blocking): one path can list several drivers in
/etc/path_to_inst (i915 and i915n), in an order a rewrite can change.
drmadm takes the one that is loaded and reports a link with more than
one. coldboot fails when the boot that arm armed has no trace, and with
REQUIRE_TRACE=1 for any boot.

README, drmadm(8): several path_to_inst drivers for one node 56005459aa

README: G10 final-revision run 5f8016c617

toasterson changed target branch from g7-coexist to main 2026-10-05 16:47:09 +00:00

toasterson merged commit 7e542bd38f into main 2026-10-05 16:47:12 +00:00

toasterson referenced this pull request from a commit 2026-10-05 16:47:15 +00:00

Merge pull request 'G10: keep the DRM nodes at minor_perm across boots (logindevperm, drm-perms service)' (#12) from g10-perms into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

illumos/illumos-drm!12

No description provided.