Allocate device resources only from the driver's own node (OwnNode) #2

Merged

forge merged 6 commits from dma-own-node into main 2026-09-28 18:10:37 +00:00

Conversation 0 Commits 6 Files changed 12 +541 -67

forge commented 2026-09-28 17:24:36 +00:00

Contributor

Copy link

Stacked on #1 (drm-r0, clone opens and devmap): the base is drm-r0, so this diff shows only the own-node change. Merge #1 first.

The panic this prevents

On 2026-09-28 a test kernel (illumos cda5b9a49f) panicked with BAD TRAP: type=e (#pf Page fault) ... addr=0 ... NULL pointer dereference, pc=0, inside ddi_dma_addr_bind_handle. The illumos-drm rdrmnull DMA drill had allocated its DMA handles against ddi_root_node(9F) instead of its own node. Rootnex accepts the root as its own requester, so the allocation succeeds. The bind then calls the root node's devi_bus_dma_bindfunc, which is never filled because the root has no parent. ddi_dma_alloc_handle(9F) requires "the device's dev_info structure" and says "a DMA handle must only be used by the device for which it was allocated".

• Analysis with the mdb transcript: workspace note notes/illumos-drm-dma-panic-20260928.md
• Drill and reproduction: illumos/illumos-drm#3

The safe layer reached the root node only through unsafe { DevInfo::from_raw(...) }. However, DevInfo is Copy + Send + Sync and names no owner, so safe code could still pass another instance's node, and no documentation stated the own-node rule.

The rule (docs/DESIGN.md, new section 5.12 "Panics become types")

Every kernel panic traced to DDI misuse must become impossible in the safe layer. Each such panic gets four things, made together:

1. a type or capability that makes the misuse unrepresentable
2. a runtime guard wherever the type can be bypassed with unsafe
3. regression tests: a compile_fail doctest (error code pinned, with a compiling twin) and an rselftest check that cannot panic even if the guard is missing
4. a row in the new "Panics prevented" table

This panic is the first row.

API change

• OwnNode (crates/ddi/src/device.rs) is the capability for the driver's own attached node.
  • It is neither Copy nor Clone. It derefs to DevInfo for queries.
  • Only the module! glue creates it, in attach(9E) with DDI_ATTACH. The glue also records ddi_driver_major there, for the guard.
  • A foreign node needs unsafe OwnNode::from_raw, whose contract names the rule. Its only legitimate use is testing the guard.
• Breaking: Driver::attach(node: OwnNode, ctx) replaces attach(dev: DevInfo, ctx). A driver keeps node in its instance state if it allocates device resources later, as the DRM core does. rnull and rselftest are adapted.
• Guard: OwnNode::is_plausibly_own(), crate-private, refuses a node without a parent (the root, detected with ddi_get_parent). It also refuses a node whose ddi_driver_major differs from the major recorded at attach.
  • The major is kept in a crate static. Each Rust driver module links its own copy of ddi, so the value is per module.
  • The guard runs before the kernel is called.
  • It cannot detect another instance of the same driver, or a node leaked past detach. Section 5.12 says so.
• DmaHandle::new / try_new take &OwnNode.
  • A node the guard refuses gets the new DmaError::NotOwnNode.
  • The docs quote 9F and note that a pseudo node gets BadAttr, which is observed behaviour.
• impl From<DmaError> for Errno is new:
  • NotOwnNode → ENXIO
  • BadAttr and the other invalid requests → EINVAL
  • NoResources and TooBig → ENOMEM
  • NoMapping → EFAULT
  • InUse and Locked → EBUSY
  • everything else → EIO
• No new -sys items: ddi_get_parent, ddi_driver_major and ddi_root_node were already bound.

Per-API decisions

API	Takes	Why
DmaHandle::new, try_new	&OwnNode + guard (NotOwnNode)	The panic itself: 9F requires the device's own node.
Interrupts::alloc, try_alloc	&OwnNode + guard (IntrError::Invalid)	ddi_intr_alloc → i_ddi_intr_ops dereferences the node's parent (NEXUS_HAS_INTR_OP(NULL)), so the root node panics too. The vectors belong to the requester.
RegMap::map, RegBlock::map	&OwnNode + guard (DdiError)	ddi_regs_map_setup → ddi_map dereferences devi_parent, so the root node panics. A foreign node would map another device's registers.
PciConfig::setup	&OwnNode + guard (DdiError)	pci_config_setup is ddi_regs_map_setup(dip, 0, ...), the same path.
intr_types, intr_count, intr_available, nregs, regsize, props, names, instance, logging	DevInfo	Queries. A root DevInfo still needs unsafe DevInfo::from_raw, whose contract is unchanged.
SoftInterrupt::add	DevInfo	ddi_intr_add_softint uses the node only for its name (add_avsoftintr) and does not go through the parent.
TaskQueue::new	DevInfo	ddi_taskq_create uses the node only to build the queue name.
LdiIdent::from_dip	DevInfo	Identity (major and name) only, with no parent operation. Follow-up below.
DevInfo::create_minor_node	DevInfo	No parent operation and no known panic. A foreign node's minor would outlive our detach. Follow-up below.
Soft state	glue only	Not reachable from driver code.
kstats	not node-based	kstat_create takes module name and instance.
devmap setup	not wrapped	devmap_umem_setup(9F) takes a dip but is called raw by drivers (illumos-drm). Follow-up below.

Tests and evidence

Logs are in the workspace under logs/hephaistos/illumos-drm/20260928-ddi-own-node-*.

• Host (macOS, Rust 1.97.1):
  • cargo check, clippy -D warnings, fmt --check and doc -D warnings on x86_64-unknown-illumos: clean.
  • cargo test -p ddi --no-default-features --lib: 6 passed (guard decision, errno mapping, attribute checks).
  • Doctests on x86_64-unknown-illumos: 5 passed.
  • cargo +nightly-2026-04-16 test -p ddi --no-default-features --doc: 4 compile_fail passed with their error codes enforced (E0308 DevInfo to DMA, E0507 copy, E0277 clone, E0133 safe from_raw), plus the compiling twin.
  • Stable rustdoc ignores error codes. Nightly caught a clone test that failed for the wrong reason (node.clone() cloned the reference), and that test was fixed.
• CI: the hosted job now runs cargo test -p ddi --no-default-features --lib and the illumos-target doctests. No CI step ran tests before this. The --lib run on the Linux runner has not been verified yet.
• cargo xtask lint-man --coverage (gate 0371167eed): 0 errors, 1183/1183.
• cargo xtask kernel-check against the guest kernel (cda5b9a49f): no size, offset or symbol disagreed.
  • ddi-sys 983/984 symbols, with the known ddi_ncpus_expected note.
  • ddi-gld-sys is 14/14 once misc/gld is loaded.
  • The 16 remaining errors are all ddi-ktest-sys symbols. drv/ktest is not installed on the image, as in section 8.1.
• rselftest on a disposable Hephaistos guest (golden overlay, illumos cda5b9a49f): 38/38 checks at attach and 7/7 LDI checks, 0 failed, no panic, clean rem_drv.
  • The three new checks:
    • "dma own pseudo node is BadAttr"
    • "dma root node refused by guard" (new and try_new)
    • "dma other driver's node refused by guard" (the pseudo nexus node, which exercises the major check)
  • They only allocate and never bind. A handle allocated against expectation is freed at once.
  • Negative control: a throwaway build with the guard patched out (never committed) reported checks 36 and 37 as FAILED (36 passed, 2 failed) and the system did not panic.
• illumos-drm (#3, pinned to b24c307, tested on the same guest type):
  • dma-mode="none" and "self" both gave R0 105, R1 153 and probes 5 and 4 PASS with no test FAIL. self leaves the known path_to_inst residue.
  • The panic-drill build with root-node still panics with the same panic string and root-node address, and savecore keeps the dump.
  • The default build refuses root-node at attach without a panic.
• The last commit (dba0697) changes docs only.

Follow-ups

• LdiIdent::from_dip, DevInfo::create_minor_node and property updates could move to OwnNode. They tie identity, minor nodes or properties to a node, but no panic is known, so they were left out to keep this change small.
• Wrap devmap_umem_setup(9F) and devmap_devmem_setup(9F) behind OwnNode, since drivers call them raw today.
• The compile_fail error codes are only enforced on nightly. The CI's native illumos job has the pinned nightly and could run the doctests.
• The "past detach" misuse is not expressible as a type while Driver::Instance: 'static (section 5.12).
• Kernel side: ddi_dma_addr_bind_handle could refuse a handle whose rdip is the root instead of calling through NULL. That is a separate, local-only gate patch under the owner's rule and is not part of this PR.

Stacked on #1 (`drm-r0`, clone opens and devmap): the base is `drm-r0`, so this diff shows only the own-node change. Merge #1 first. ## The panic this prevents On 2026-09-28 a test kernel (illumos `cda5b9a49f`) panicked with `BAD TRAP: type=e (#pf Page fault) ... addr=0 ... NULL pointer dereference`, `pc=0`, inside `ddi_dma_addr_bind_handle`. The illumos-drm `rdrmnull` DMA drill had allocated its DMA handles against `ddi_root_node(9F)` instead of its own node. Rootnex accepts the root as its own requester, so the allocation succeeds. The bind then calls the root node's `devi_bus_dma_bindfunc`, which is never filled because the root has no parent. `ddi_dma_alloc_handle(9F)` requires "the device's dev_info structure" and says "a DMA handle must only be used by the device for which it was allocated". - Analysis with the mdb transcript: workspace note `notes/illumos-drm-dma-panic-20260928.md` - Drill and reproduction: illumos/illumos-drm#3 The safe layer reached the root node only through `unsafe { DevInfo::from_raw(...) }`. However, `DevInfo` is `Copy + Send + Sync` and names no owner, so safe code could still pass another instance's node, and no documentation stated the own-node rule. ## The rule (docs/DESIGN.md, new section 5.12 "Panics become types") Every kernel panic traced to DDI misuse must become impossible in the safe layer. Each such panic gets four things, made together: 1. a type or capability that makes the misuse unrepresentable 2. a runtime guard wherever the type can be bypassed with `unsafe` 3. regression tests: a `compile_fail` doctest (error code pinned, with a compiling twin) and an `rselftest` check that cannot panic even if the guard is missing 4. a row in the new "Panics prevented" table This panic is the first row. ## API change - **`OwnNode`** (`crates/ddi/src/device.rs`) is the capability for the driver's own attached node. - It is neither `Copy` nor `Clone`. It derefs to `DevInfo` for queries. - Only the `module!` glue creates it, in `attach(9E)` with `DDI_ATTACH`. The glue also records `ddi_driver_major` there, for the guard. - A foreign node needs `unsafe OwnNode::from_raw`, whose contract names the rule. Its only legitimate use is testing the guard. - **Breaking:** `Driver::attach(node: OwnNode, ctx)` replaces `attach(dev: DevInfo, ctx)`. A driver keeps `node` in its instance state if it allocates device resources later, as the DRM core does. `rnull` and `rselftest` are adapted. - **Guard:** `OwnNode::is_plausibly_own()`, crate-private, refuses a node without a parent (the root, detected with `ddi_get_parent`). It also refuses a node whose `ddi_driver_major` differs from the major recorded at attach. - The major is kept in a crate static. Each Rust driver module links its own copy of `ddi`, so the value is per module. - The guard runs before the kernel is called. - It cannot detect another instance of the same driver, or a node leaked past detach. Section 5.12 says so. - **`DmaHandle::new` / `try_new`** take `&OwnNode`. - A node the guard refuses gets the new `DmaError::NotOwnNode`. - The docs quote 9F and note that a pseudo node gets `BadAttr`, which is observed behaviour. - **`impl From<DmaError> for Errno`** is new: - `NotOwnNode` → `ENXIO` - `BadAttr` and the other invalid requests → `EINVAL` - `NoResources` and `TooBig` → `ENOMEM` - `NoMapping` → `EFAULT` - `InUse` and `Locked` → `EBUSY` - everything else → `EIO` - No new `-sys` items: `ddi_get_parent`, `ddi_driver_major` and `ddi_root_node` were already bound. ## Per-API decisions | API | Takes | Why | | --- | --- | --- | | `DmaHandle::new`, `try_new` | `&OwnNode` + guard (`NotOwnNode`) | The panic itself: 9F requires the device's own node. | | `Interrupts::alloc`, `try_alloc` | `&OwnNode` + guard (`IntrError::Invalid`) | `ddi_intr_alloc` → `i_ddi_intr_ops` dereferences the node's parent (`NEXUS_HAS_INTR_OP(NULL)`), so the root node panics too. The vectors belong to the requester. | | `RegMap::map`, `RegBlock::map` | `&OwnNode` + guard (`DdiError`) | `ddi_regs_map_setup` → `ddi_map` dereferences `devi_parent`, so the root node panics. A foreign node would map another device's registers. | | `PciConfig::setup` | `&OwnNode` + guard (`DdiError`) | `pci_config_setup` is `ddi_regs_map_setup(dip, 0, ...)`, the same path. | | `intr_types`, `intr_count`, `intr_available`, `nregs`, `regsize`, props, names, `instance`, logging | `DevInfo` | Queries. A root `DevInfo` still needs `unsafe DevInfo::from_raw`, whose contract is unchanged. | | `SoftInterrupt::add` | `DevInfo` | `ddi_intr_add_softint` uses the node only for its name (`add_avsoftintr`) and does not go through the parent. | | `TaskQueue::new` | `DevInfo` | `ddi_taskq_create` uses the node only to build the queue name. | | `LdiIdent::from_dip` | `DevInfo` | Identity (major and name) only, with no parent operation. Follow-up below. | | `DevInfo::create_minor_node` | `DevInfo` | No parent operation and no known panic. A foreign node's minor would outlive our detach. Follow-up below. | | Soft state | glue only | Not reachable from driver code. | | kstats | not node-based | `kstat_create` takes module name and instance. | | devmap setup | not wrapped | `devmap_umem_setup(9F)` takes a dip but is called raw by drivers (illumos-drm). Follow-up below. | ## Tests and evidence Logs are in the workspace under `logs/hephaistos/illumos-drm/20260928-ddi-own-node-*`. - **Host (macOS, Rust 1.97.1):** - `cargo check`, `clippy -D warnings`, `fmt --check` and `doc -D warnings` on `x86_64-unknown-illumos`: clean. - `cargo test -p ddi --no-default-features --lib`: 6 passed (guard decision, errno mapping, attribute checks). - Doctests on `x86_64-unknown-illumos`: 5 passed. - `cargo +nightly-2026-04-16 test -p ddi --no-default-features --doc`: 4 `compile_fail` passed with their error codes enforced (E0308 `DevInfo` to DMA, E0507 copy, E0277 clone, E0133 safe `from_raw`), plus the compiling twin. - Stable rustdoc ignores error codes. Nightly caught a clone test that failed for the wrong reason (`node.clone()` cloned the reference), and that test was fixed. - **CI:** the hosted job now runs `cargo test -p ddi --no-default-features --lib` and the illumos-target doctests. No CI step ran tests before this. The `--lib` run on the Linux runner has not been verified yet. - **`cargo xtask lint-man --coverage`** (gate `0371167eed`): 0 errors, 1183/1183. - **`cargo xtask kernel-check`** against the guest kernel (`cda5b9a49f`): no size, offset or symbol disagreed. - `ddi-sys` 983/984 symbols, with the known `ddi_ncpus_expected` note. - `ddi-gld-sys` is 14/14 once `misc/gld` is loaded. - The 16 remaining errors are all `ddi-ktest-sys` symbols. `drv/ktest` is not installed on the image, as in section 8.1. - **rselftest** on a disposable Hephaistos guest (golden overlay, illumos `cda5b9a49f`): 38/38 checks at attach and 7/7 LDI checks, 0 failed, no panic, clean `rem_drv`. - The three new checks: - "dma own pseudo node is BadAttr" - "dma root node refused by guard" (`new` and `try_new`) - "dma other driver's node refused by guard" (the pseudo nexus node, which exercises the major check) - They only allocate and never bind. A handle allocated against expectation is freed at once. - **Negative control:** a throwaway build with the guard patched out (never committed) reported checks 36 and 37 as FAILED (36 passed, 2 failed) and the system did not panic. - **illumos-drm** (#3, pinned to `b24c307`, tested on the same guest type): - `dma-mode="none"` and `"self"` both gave R0 105, R1 153 and probes 5 and 4 PASS with no test FAIL. `self` leaves the known `path_to_inst` residue. - The `panic-drill` build with `root-node` still panics with the same panic string and root-node address, and savecore keeps the dump. - The default build refuses `root-node` at attach without a panic. - The last commit (`dba0697`) changes docs only. ## Follow-ups - `LdiIdent::from_dip`, `DevInfo::create_minor_node` and property updates could move to `OwnNode`. They tie identity, minor nodes or properties to a node, but no panic is known, so they were left out to keep this change small. - Wrap `devmap_umem_setup(9F)` and `devmap_devmem_setup(9F)` behind `OwnNode`, since drivers call them raw today. - The `compile_fail` error codes are only enforced on nightly. The CI's native illumos job has the pinned nightly and could run the doctests. - The "past detach" misuse is not expressible as a type while `Driver::Instance: 'static` (section 5.12). - Kernel side: `ddi_dma_addr_bind_handle` could refuse a handle whose `rdip` is the root instead of calling through NULL. That is a separate, local-only gate patch under the owner's rule and is not part of this PR.

forge added 6 commits 2026-09-28 17:24:37 +00:00

Allocate DMA handles only for the driver's own node ... 606c0219bb

On 2026-09-28 a driver panicked a test kernel by allocating its DMA
handles against ddi_root_node(9F) instead of its own node: rootnex
accepted the root as its own requester, and ddi_dma_addr_bind_handle(9F)
then called the root node's devi_bus_dma_bindfunc, which is never filled
because the root has no parent. ddi_dma_alloc_handle(9F) requires "the
device's dev_info structure", and a handle "must only be used by the
device for which it was allocated".

The safe layer only reached the root node through an unsafe
DevInfo::from_raw, but DevInfo is Copy and names no owner, so safe code
could still pass another instance's node. This makes the rule a type:

- OwnNode is the capability for the driver's own attached node. Only the
  attach glue creates it, from the node of attach(9E) with DDI_ATTACH,
  and hands it to Driver::attach by value; the driver keeps it in its
  instance state. It is neither Copy nor Clone, dereferences to its
  DevInfo for queries, and a foreign node needs the unsafe
  OwnNode::from_raw. Driver::attach now takes an OwnNode.
- DmaHandle::new and try_new take &OwnNode.
- Runtime guard for the unsafe bypass: before ddi_dma_alloc_handle, a
  node without a parent (the root) or bound to another driver than the
  major recorded at attach is refused with the new DmaError::NotOwnNode.
- From<DmaError> for Errno, with NotOwnNode as ENXIO.

compile_fail doctests (with compiling twins) cover a DevInfo for DMA,
copying, cloning and a safe from_raw; host tests cover the guard's
decision and the errno mapping. rnull and rselftest take the new attach
argument.

Take the own node for interrupts, register mappings and PCI config ... ddb292cad7

ddi_intr_alloc(9F), ddi_regs_map_setup(9F) and pci_config_setup(9F)
ask the node's parent nexus for the resource. For the root node,
i_ddi_intr_ops and ddi_map dereference its NULL parent, so the misuse
that panicked the DMA bind panics these as well, and all of them tie the
resource to the requesting device. Interrupts::alloc and try_alloc,
RegMap::map, RegBlock::map and PciConfig::setup therefore take &OwnNode
and refuse a node that fails the own-node guard (IntrError::Invalid,
DdiError) before calling the kernel.

The queries (intr_types, intr_count, nregs, regsize) stay on DevInfo.
SoftInterrupt::add, TaskQueue, LdiIdent::from_dip, minor nodes and
properties keep DevInfo: they use the node for its name, identity or
property list and do not go through the parent nexus.

Check the DMA own-node guard in rselftest ... cdbe146e53

Three new checks at attach. The driver's own pseudo node reaches the
kernel and gets DDI_DMA_BADATTR, because the pseudo nexus does no DMA.
An OwnNode made with the unsafe from_raw for ddi_root_node(9F) is
refused with DmaError::NotOwnNode by new and try_new, and so is the
pseudo nexus node, which has a parent but another driver's major.

The checks only allocate and never bind. A handle allocated against
expectation is freed at once by its drop and reported as a failure, and
freeing a root-node handle works, so the checks cannot panic the system
even if the guard were missing.

Run the host tests and the compile_fail doctests in CI ... ef8984ae7a

No CI step ran any test. The hosted job now runs the ddi unit tests on the host and the doctests against the illumos target, both without the panic handler, which would clash with std's. The doctests only compile, so the illumos target works on the Linux runner.

Make panics become types, starting with the DMA own-node rule ... b24c307298

Section 5.12 states the standing rule: every kernel panic traced to DDI
misuse becomes impossible in the safe layer, through a type or
capability, a runtime guard for the unsafe bypass, a compile_fail
doctest and an rselftest check, and a row in the new "Panics prevented"
table. The first row is the DMA root-node panic of 2026-09-28.

Sections 5.2, 5.5 and 5.11 describe OwnNode: Driver::attach receives
it, the resource constructors take it, DevInfo stays for queries, and
OwnNode::from_raw is the unsafe bypass.

Record the rselftest own-node checks and their results ... dba0697bc4

rselftest now runs 38 checks at attach and 7 LDI checks. All 45 passed on 28 September 2026 on a Hephaistos guest with illumos cda5b9a49f, and a build with the guard patched out reported the two guard checks as failed without panicking. The README mentions the panics-become-types rule.

forge referenced this pull request from illumos/illumos-drm 2026-09-28 17:25:04 +00:00

R1 DMA drill: reproduce the root-node DMA panic, fix the /devices snapshot race #3

forge changed target branch from drm-r0 to main 2026-09-28 18:10:23 +00:00

forge merged commit da82578cf6 into main 2026-09-28 18:10:37 +00:00

forge referenced this pull request from a commit 2026-09-28 18:10:38 +00:00

Merge pull request #2 from dma-own-node: make DMA misuse against foreign nodes impossible

forge referenced this pull request 2026-09-28 18:35:58 +00:00

Loose ends: private-tier consumer, OwnNode for minor nodes, properties and LDI, nightly doctests in CI #3

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-ddi!2

No description provided.