# DF-5632 — `iicbus_add_child` registered with a 4-arg/`int` prototype against DragonFly's 5-arg/`device_t` `bus_add_child` interface → argument+return skew → out-of-bounds kernel write in `devclass_add_device`

* File under audit: `sys/bus/iicbus/iicbus.c` (pass 2)
* Class: kernel memory corruption (OOB write) via silently type-confused kobj method registration
* Severity: Medium (trigger = `kldload` of any identify-based iicbus client driver — root-gated; in-tree `iic`/`iicsmb` are only masked by an accident of pre-created children)
* Verdict: **REPRODUCED** (two independent stock triggers, both ending in `Fatal trap 12` at `devclass_add_device+0xf6`), fix validated

## The defect

DragonFly's `bus_add_child` interface is (sys/kern/bus_if.m:128-134):

```m
METHOD device_t add_child {
	device_t bus;
	device_t parent;
	int order;
	const char *name;
	int unit;
};
```

Five arguments, returns the new `device_t`. Every in-tree caller uses
`(parent, parent, order, name, unit)` — e.g. `nexus.c:251`, `isahint.c:55`,
**`iic.c:109`**, **`iicsmb.c:141`** — and `nexus_add_child`
(sys/platform/pc64/x86_64/nexus.c:90,293) shows the correct implementation
shape (5 args, returns child, allocates ivars).

`iicbus.c` registers a FreeBSD-4-era 4-arg function returning `int`
(`iicbus.c:57`, `:164-171`):

```c
static int iicbus_add_child(device_t dev, int order, const char *name, int unit);
...
DEVMETHOD(bus_add_child, iicbus_add_child),          /* iicbus.c:66 */
...
static int
iicbus_add_child(device_t dev, int order, const char *name, int unit)
{
	device_add_child_ordered(dev, order, name, unit);
	bus_generic_attach(dev);
	return (0);
}
```

`DEVMETHOD` stores the raw function pointer (no compile-time signature
check), so the kobj dispatcher calls it with 5 register arguments and
interprets `EAX` as a `device_t`. On x86-64 the callee therefore reads:

| iicbus_add_child param | actually receives |
|---|---|
| `order` | the `parent` device_t, truncated to `int` |
| `name`  | the `order` int (0 → `(char *)0` = NULL) |
| `unit`  | the `name` string literal address, truncated to `int` |
| return  | `0` → `NULL` handed to every caller expecting the child |

and then calls `device_add_child_ordered(dev, (int)parent, NULL, (int)name_addr)`.

## Why it corrupts memory

`make_device(parent, NULL, garbage_unit)` (subr_bus.c) creates a classless
child with `unit = (int)(intptr_t)"name"` — a low-32-bits kernel rodata
address, in practice a **negative** int. When that ghost is probed (by the
`bus_generic_attach(dev)` inside `iicbus_add_child` itself, or by the
`bus_generic_driver_added` probe loop), `device_probe_child()` →
`device_set_devclass(child, driver->name)` → `devclass_add_device()` runs:

* `devclass_alloc_unit()` (subr_bus.c:~2460): a *negative* wired unit skips
  both the collision branch (`unit >= 0 && ...`) and the table-extension
  branch (`unit >= dc->maxunit`) and is returned **unchanged**;
* `devclass_add_device()` (subr_bus.c:~2420): `dc->devices[dev->unit] = dev`
  — an **8-byte write of a kernel heap pointer below the device array**.

## Reproduction (guest, stock `/boot/kernel/iicbus.ko`)

Rig: `df5632_rig.c` — fake GPIO bridge under nexus hosting the production
chain `fakebb0 → iicbb0 (iicbb.ko) → iicbus0 (iicbus.ko)`, plus a client
driver `df5632k` whose `identify` is a verbatim copy of the `iic.c:104-110`
pattern. Two independent triggers:

1. **Module-load trigger** (`panic_skew_stock.txt`): loading the rig module
   registers `df5632k_driver` on the attached iicbus0 →
   `bus_generic_driver_added` → `df5632k_identify` → `BUS_ADD_CHILD(iicbus0,
   iicbus0, 0, "df5632k", -1)` → skew → ghost child → probe →
   `Fatal trap 12 ... supervisor write data, page not present`,
   `fault virtual address = 0xfffffffc131cb558`,
   `Stopped at devclass_add_device+0xf6: movq %r14,(%rdx,%rax,1)`,
   current process = the `kldload`. *This is exactly what happens to any
   driver author who follows the documented identify pattern
   (device_if.m:99-101) for a new iicbus client.*
2. **API-contract trigger** (`panic_skew_addchild.txt`): with the bus up and
   healthy (`STAT: nchildren=3`, iicbus0 `state=3` attached), one
   `BUS_ADD_CHILD(bus, bus, 0, "df5632c", -1)` — the call every identify
   method makes — panics identically
   (`fault virtual address = 0xfffffffc131cc228`, same stopped PC).

The only reason the in-tree `iic.ko`/`iicsmb.ko` do not hit this today is
that `iicbus_attach()` pre-creates the `ic`/`iic`/`iicsmb` trio
(iicbus.c:144-146), so those drivers' identify guards
(`device_find_child(...) == NULL`) never fire — an accident, not a
protection: any client whose rendezvous child is missing (deleted child,
out-of-tree driver, future driver, DF-5633's ghost-populated devclass corner
cases) takes the OOB write.

## Fix (validated)

`fix.diff` re-types `iicbus_add_child` to the interface contract
(`static device_t iicbus_add_child(device_t bus, device_t parent, int order,
const char *name, int unit)`), returns the created child (and allocates its
`struct iicbus_ivar`, see DF-5634), and drops the redundant
`bus_generic_attach()` (the `bus_generic_driver_added` probe loop already
runs after identify; `nexus_add_child` does not re-attach either).

With only `iicbus.ko` rebuilt from this diff (`run_d_fixed.log`):

* `dfctl addchild`: `BUS_ADD_CHILD` **returned 0xfffff801175bc9c0** (a real
  device_t) and the child list shows `child[3] devclass=df5632c unit=0` —
  the requested rendezvous, created correctly;
* no panic at any point; module load, ioctls, cycling and unload all clean.