DF-5528 — fwohci_pci_detach() swallows child-detach failure, force-deletes the firewire child, and returns unconditional success

File: sys/bus/firewire/fwohci_pci.c (pass 2, GLM 5.3 re-audit) Class: driver lifecycle / error-handling (CWE-754, latent CWE-672 downstream) Severity: Low — latent trigger, requires FireWire OHCI hardware + privileged unload (or CardBus surprise removal)

TL;DR

fwohci_pci_detach() (sys/bus/firewire/fwohci_pci.c:341-386) discards the return value of bus_generic_detach(self) (line 350), unconditionally force-deletes the firewire child device (lines 351-354), and always return 0 (line 385). If any device on the fwohci→firewire→{sbp,fwe,dcons_crom} chain ever vetoes detach (returns EBUSY), the parent nonetheless releases the IRQ, unmaps the BAR and frees the OHCI DMA rings while the firewire child remains DS_ATTACHED and orphaned with live callouts — a use-after-free / page-0-MMIO-write panic chain, and kldunload reports success with live references left behind.

Latent today: in this tree no firewire-bus child can currently veto (sbp_detach sys/dev/disk/sbp/sbp.c:2016-2058, fwe_detach sys/dev/netif/fwe/if_fwe.c:284-295 and dcons_crom_detach sys/dev/misc/dcons/dcons_crom.c:190-207 all return 0 unconditionally; fwdev_destroydev sys/bus/firewire/fwdev.c:809-817 likewise). The defect is a structural trap, not a live regression — hence Low/untested.

How to run / what "reproducing" would mean

1. Requires a machine with a real FireWire OHCI PCI/CardBus controller (PCI class 0c/10/10) — there is no such device in the QEMU guest (see run.log: pciconf -l shows only i440fx/virtio devices).
2. Build the fwohci + firewire + sbp KLDs, attach storage over FireWire so an sbp target exists, then make sbp_detach() return EBUSY (upstream FreeBSD versions of sbp had such guards; this tree does not — add one to test).
3. kldunload if_fwohci.ko → observe: "detached" reported, unload completes, firewire watchdog callout (firewire.c:375-376, stopped only inside the never-completed firewire_detach at firewire.c:468-471) fires and executes OWRITE() through the zeroed sc->bst/sc->bsh (fwohci_pci.c:378-379) → kernel page-0 write → panic.
4. With the fix (fix.diff), the unload fails with the child's error code and nothing is torn down.

run.sh documents the hardware-gating check performed on the audit guest.

Artifacts

• lifecycle_trace.md — line-cited callgraph of both the veto and non-veto paths
• check_guest.sh / run.sh + run.log — proof the audit guest has no OHCI device
• fix.diff — git-apply-able fix (propagate bus_generic_detach error; never force-delete a child that vetoed; do not report unconditional success)
• verdict.json — machine verdict (status untested, hardware-gated)

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

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_stops 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_xfers on the (already fwohci_stoped, 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.