DF-3812 — isp_add_wwn_entry / isp_del_wwn_entry index FCPARAM(chan) without any channel bounds check; chan is a raw firmware byte

Files

• sys/dev/disk/isp/isp_library.c:2391-2398 (isp_add_wwn_entry)
• sys/dev/disk/isp/isp_library.c:2517-2529 (isp_del_wwn_entry)
• sys/dev/disk/isp/isp_library.c:2560-2584 (isp_del_all_wwn_entries — adjacent off-by-one chan > isp->isp_nchan at :2580 admits chan == nchan)

Build / Run / Expected

Two parts:

1. Offset-arithmetic demonstrator (runs anywhere, executed on the audit guest as an unprivileged user): cc -Wall -o vpidx vpidx_oob_arith.c && ./vpidx Expected (see build.log, executed on DragonFly 6.5-DEVELOPMENT): sizeof(fcparam) replica lower bound = 69,696 B; isp_param is nchan ∈ {1,2} fcparams (isp_pci.c:824-832), so an in_vpidx of e.g. 52 indexes 3.55 MB past the block and 255 indexes 17.77 MB past it.
2. In-vivo trigger (NOT executed — the guest has no QLogic HBA): target-mode ISP FC adapter with MULTI-ID firmware receives an ELS (PLOGI/PRLI) whose firmware-generated IN24XX_ELS_RCVD notify IOCB carries in_vpidx >= isp_nchan; the driver then reads/writes kernel heap far outside isp_param (list under "Impact").

Root cause chain

/* isp_freebsd.c:2626-2627 */  int chan = ISP_GET_VPIDX(isp, inot->in_vpidx);
/* ispmbox.h:861-862 */        #define ISP_GET_VPIDX(isp, tag) \
                                  (ISP_CAP_MULTI_ID(isp) ? tag : 0)   /* raw 8-bit fw field */
/* isp_freebsd.c:2677 */       isp_add_wwn_entry(isp, chan, wwn, nphdl, portid);
/* isp_library.c:2398 */       fcp = FCPARAM(isp, chan);
/* ispvar.h:644 */             #define FCPARAM(isp, chan) (&((fcparam *)(isp)->isp_param)[(chan)])

Every sibling in the same library validates first — isp_find_pdb_by_wwn (isp_library.c:2304 if (chan < isp->isp_nchan)), isp_find_pdb_by_loopid (:2327), isp_find_pdb_by_sid (:2350), isp_fc_change_role (:512) — but isp_add_wwn_entry/isp_del_wwn_entry dereference FCPARAM(isp, chan) unvalidated. isp_del_all_wwn_entries checks chan > isp->isp_nchan (:2580) instead of >=, admitting chan == nchan (one-past-end fcparam).

Through the OOB fcp the function then performs, on attacker/firmware- influenced data: - reads fcp->isp_tgt_map[nphdl] (isp_library.c:2407; nphdl is the firmware 16-bit N-port handle, bounded only against MAX_NPORT_HANDLE=2048 relative to the already-OOB pointer); - writes fcp->isp_tgt_map[nphdl] = i + 1 (:2497); - writes lp->target_mode = 1, lp->handle = nphdl, lp->portid = s_id, lp->port_wwn = ini (:2491-2496) into fcp->portdb[...] — i.e. an 8-byte remote-controlled WWN plus 32-bit port ID written at chan * sizeof(fcparam) + <portdb offset> + entry*fcportdb; - dispatches isp_async(NT_ARRIVED) with the OOB-derived fields (:2510).

Threat model

Same trust boundary as previously accepted DF-2471 (card-reported channel in isp.c async handler) and DF-3582/3583 (card-controlled fields): the field is produced by the adapter firmware/DMA, i.e. a compromised or buggy HBA (or PCIe DMA attacker) fully controls it; a remote FC peer controls the ELS traffic that generates the notification but normally only sees its own valid VP index. Legit firmware reports only configured VPs, which is why this is Medium, not High: it is a hardening hole in the kernel's trust of the card, with a multi-field OOB write primitive behind it.

Impact

Kernel heap OOB read + OOB write of remote-controlled data (WWN/portid), offset controlled in units of ~68 KB up to ~17.8 MB past a 68–139 KB kmalloc block — arbitrary heap corruption on target-mode FC with MULTI-ID firmware. Guest has no QLogic hardware, so the driver path could not be exercised: classification inconclusive (source-certain, untriggerable here).

DF-3812 — VERDICT

status: inconclusive (poc enum) — no QLogic HBA on the audit guest; the code defect is certain from source, and the index arithmetic was demonstrated by a userland replica built and run on the guest (build.log).

What was verified

1. The missing guard, precisely. isp_add_wwn_entry (isp_library.c:2391) and isp_del_wwn_entry (:2517) take chan straight into FCPARAM(isp, chan) (:2398, :2529) with no bound check, while their four siblings in the same file all check chan < isp->isp_nchan (isp_find_pdb_by_wwn :2304, isp_find_pdb_by_loopid :2327, isp_find_pdb_by_sid :2350, isp_fc_change_role :512). The asymmetry is the bug — it is not plausibly intentional.
2. The tainted source. On the IN24XX_ELS_RCVD path, chan = ISP_GET_VPIDX(isp, inot->in_vpidx) (isp_freebsd.c:2627) and ISP_GET_VPIDX is a pass-through of the raw 8-bit queue-entry field whenever the firmware advertises MULTI-ID (ispmbox.h:861-862, ISP_CAP_MULTI_ID at ispmbox.h:858-859). PLOGI/PRLI subcodes then call isp_add_wwn_entry(isp, chan, wwn, nphdl, portid) (isp_freebsd.c:2677); LOGO calls isp_del_wwn_entry(isp, chan, ...) (:2650); PORT_LOGOUT calls it with ISP_GET_VPIDX(...) inline (:2702-2703).
3. The allocation geometry. isp->isp_param = kmalloc(sizeof(fcparam) * isp->isp_nchan) (isp_pci.c:824-832); isp_nchan is 1 or 2 (isp_pci.c:662/732/749, isp.c:1105). fcparam embeds uint16_t isp_tgt_map[2048] (ispvar.h:496) and fcportdb_t portdb[512] (MAX_FC_TARG=512, ispvar.h:66) — ≥ ~68 KB per channel. Demonstrated on the guest (build.log): in_vpidx 52 → 3.55 MB past the block; 255 → 17.77 MB past.
4. The write set through the OOB fcp. isp_library.c:2407 (tgt_map read), :2497 (fcp->isp_tgt_map[nphdl] = i + 1), :2491-2496 (portdb entry: target_mode=1, handle=nphdl, portid=s_id, port_wwn=ini — 8-byte remote-controlled value), :2510 (async dispatch). isp_del_wwn_entry writes fcp->isp_tgt_map[nphdl] = 0 (:2546) through the same OOB pointer.
5. Adjacent off-by-one. isp_del_all_wwn_entries gates with if (chan > isp->isp_nchan) (isp_library.c:2580) — chan == nchan passes and yields the one-past-end fcparam. Current callers pass validated chans (isp_fc_change_role :518 after :512 check; recursion bounded :2574; NT loop :660-669), so it is latent today; folded into this finding.

Why not reproduced on the guest

pciconf -l | grep -ci "isp|qlogic" = 0 on the DragonFly QEMU guest — no QLogic HBA, no ISP attachment; the notify-IOCB path cannot be entered. Per the task's honesty guidance the verdict is inconclusive: certain by source, untriggerable on available hardware. uid0-privesc chain not pursued for the same reason (no primitive instance to groom).

Consistency with the DB

Same class/trust boundary as accepted DF-2471 (isp.c async-event channel) and DF-3582/3583 (card-controlled fields). Rated Medium rather than DF-2471's Low because this path performs a multi-field OOB write of remote-controlled data (WWN, port ID) plus a map write, at fully selectable ~68 KB granularity.

Fix

--- a/sys/dev/disk/isp/isp_library.c
+++ b/sys/dev/disk/isp/isp_library.c
@@ -2395,6 +2395,11 @@ void
 isp_add_wwn_entry(ispsoftc_t *isp, int chan, uint64_t ini, uint16_t nphdl, uint32_t s_id)
 {
    fcparam *fcp;
+
+   if (chan >= isp->isp_nchan) {
+       isp_prt(isp, ISP_LOGWARN, "%s: bad channel %d", __func__, chan);
+       return;
+   }

    fcp = FCPARAM(isp, chan);
@@ -2526,6 +2531,11 @@ void
 isp_del_wwn_entry(ispsoftc_t *isp, int chan, uint64_t ini, uint16_t nphdl, uint32_t s_id)
 {
    fcparam *fcp;
+
+   if (chan >= isp->isp_nchan) {
+       isp_prt(isp, ISP_LOGWARN, "%s: bad channel %d", __func__, chan);
+       return;
+   }
    isp_notify_t nt;
@@ -2577,7 +2587,7 @@ isp_del_all_wwn_entries(ispsoftc_t *isp, int chan)
        return;
    }

-   if (chan > isp->isp_nchan) {
+   if (chan >= isp->isp_nchan) {
        return;
    }

(Plus, optionally, clamp in ISP_GET_VPIDX consumers — but fixing the library functions closes every path at once.)

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.

(No fix.diff for this item: documented recommendation only - see Fix above.)