# DF-5528 — full narrative and verdict

Finding: `fwohci_pci_detach()` discards `bus_generic_detach()`'s error, force-deletes
the firewire child regardless of its detach state, and returns 0 unconditionally.
`sys/bus/firewire/fwohci_pci.c:341-386` (specifically :350, :351-354, :385).

## 1. The code

```c
341: fwohci_pci_detach(device_t self)
342: {
...
346:	crit_enter();
347:	if (sc->bsr)
348:		fwohci_stop(sc, self);
349:
350:	bus_generic_detach(self);                      /* <-- return discarded */
351:	if (sc->fc.bdev) {
352:		device_delete_child(self, sc->fc.bdev); /* <-- unconditional     */
353:		sc->fc.bdev = NULL;
354:	}
...
361:	if (sc->irq_res) { ... bus_teardown_intr ... }   /* IRQ handler gone  */
370:	if (sc->irq_res) { ... bus_release_resource ... }
375:	if (sc->bsr)   { ... bus_release_resource ...    /* BAR unmapped      */
378:		sc->bst = 0; sc->bsh = 0; }
382:	fwohci_detach(sc, self);                        /* DMA rings freed    */
383:	crit_exit();
385:	return 0;                                        /* <-- always success */
```

## 2. What newbus actually does (verified against sys/kern/subr_bus.c)

- `bus_generic_detach` (subr_bus.c) walks children calling `device_detach`;
  `device_detach` (subr_bus.c:2128-2154) returns the child error and leaves the
  child `DS_ATTACHED` on failure.
- `device_delete_child` (subr_bus.c:1284-1310) **first** calls `device_detach(child)`
  again (line 1292) — so a vetoing child re-runs its whole detach a second time on
  partially-torn-down state (`fwdev_destroydev` already ran once inside the first
  `firewire_detach`, firewire.c:462). If it vetoes again, `device_delete_child`
  returns before `TAILQ_REMOVE`/`kobj_delete` (subr_bus.c:1296-1306) — the child
  stays attached and linked into the parent's tree, but `sc->fc.bdev` was already
  NULLed by fwohci_pci.c:353.
- The parent then completes the teardown: IRQ handler unregistered (:362), IRQ
  released (:371), BAR released and `bst/bsh` zeroed (:375-379), DMA rings freed
  by `fwohci_detach` → `fwohci_db_free` (fwohci.c:764-785, fwohci.c:1158-1182),
  and reports success (:385). `kldunload` therefore completes.

## 3. Consequence chain once any grandchild vetoes

Firewire-bus device tree: fwohci (pci/cardbus) → firewire → {sbp, fwe, dcons_crom}.

1. **Live callouts on torn-down softc**: `firewire_attach` starts the self-re-arming
   watchdog `callout_reset(&sc->fc->timeout_callout, hz, firewire_watchdog, fc)`
   (firewire.c:402-403, re-arm at firewire.c:375-376). Those `callout_stop`s live
   at the END of `firewire_detach` (firewire.c:468-471) — code that a vetoing
   detach never reaches. After the parent's teardown the callout fires with
   `bst == 0 && bsh == 0` (fwohci_pci.c:378-379); any `OWRITE` in the watchdog /
   timeout path (e.g. `fwohci_start_atq` via fc->atq->start, fwohci.c:629) writes
   through a null bus handle → page-0 kernel write → panic.
2. **DMA rings freed under live users**: sbp keeps queued `fw_xfer`s on the
   (already `fwohci_stop`ed, fwohci_pci.c:348) controller; `fwohci_detach`
   (fwohci.c:773-777) frees the arr/at descriptor rings and buffers underneath
   them — post-free descriptor consumption / UAF.
3. **Orphaned attached child**: still DS_ATTACHED, parented to a detached device;
   its ivars point at `&sc->fc` whose backing resources are gone; subsequent
   module unload of `firewire` UAFs on the freed softc.
4. **Lying success**: kldunload/devctl detach report success with live references.

## 4. Why "latent" — the honesty section

In the CURRENT tree no firewire-bus child can veto:
- `sbp_detach` sys/dev/disk/sbp/sbp.c:2016-2058 → always `return 0`
- `fwe_detach` sys/dev/netif/fwe/if_fwe.c:284-295 → always `return 0`
- `dcons_crom_detach` sys/dev/misc/dcons/dcons_crom.c:190-207 → always `return 0`
- `fwdev_destroydev` sys/bus/firewire/fwdev.c:809-817 → always `return 0`
so `firewire_detach` (firewire.c:453-487) cannot currently fail, and the normal
detach path is clean (successful `device_detach` sets DS_NOTPRESENT, so the
`device_delete_child` at :352 re-detaches nothing).

The trap arms the moment any child gains an EBUSY guard (upstream FreeBSD sbp
carried such guards historically). The defect was inherited verbatim from
FreeBSD — verified against freebsd-src release/13.4.0
`sys/dev/firewire/fwohci_pci.c` whose detach also discards the error — so this is
NOT a DF-only divergence, and it is filed as robustness/hardening with a real,
line-proven panic chain behind it.

Adjacent verified-but-harmless differences vs FreeBSD 13.4 (recorded, not filed):
- DF calls `bus_teardown_intr(self, sc->irq_res, sc->ih)` with `sc->ih == NULL`
  in the bus_setup_intr-failure path (fwohci_pci.c:361-368); FreeBSD 13.4 guards
  with `if (sc->ih)`. Traced harmless: pci_teardown_intr (sys/bus/pci/pci.c:3384+)
  masks INTx then nexus_teardown_intr (nexus.c:537-544) returns -1 for a NULL
  cookie → error message only.

## 5. Verification status

- **status: untested / hardware-gated.** Reproduction requires a physical
  FireWire OHCI controller (PCI 0c/10/10) plus a patched sbp (or any vetoing
  child). The audit QEMU guest has none — see `run.log`: `pciconf -l` lists only
  i440fx/virtio functions, dmesg contains no fwohci/firewire lines, no FW KLDs
  loaded. A KLD harness of the DF-5302 class (compile-the-core) cannot express
  this bug: the defect is in the newbus parent/child detach protocol, which only
  executes against a real PCI attachment.
- Guest left untouched (read-only commands; guest_dirty=0).

## 6. Fix

`fix.diff` in this directory: capture and propagate the `bus_generic_detach`
error before touching the child or any resource, mirroring the argument
structure of the suspend path (fwohci_pci.c:395-397) which already propagates.
Fix is source-level sound; no runtime fix validation was possible (no hardware),
hence fix_status=not_testable.

## Fix status (batch validation)
Applied with the 36 sibling fix.diffs of this batch in findings/poc/_batch41/all_fixes.patch; single in-guest `make -j6 nativekernel KERNCONF=X86_64_GENERIC` completed RC=0 with zero errors under -Werror (full log: findings/poc/_batch41/fix_build8.log). Behavior A/B intentionally skipped for this Info-tier item.