⬢ DragonFlyBSD Kernel Audit
DF-4559 / fix.diff
← back to finding ↓ download raw
fix.diff — DF-4559: size sassc->targets for the full mapping-table ID space
(and make the by-handle scan see volume targets).

The mapping layer assigns IDs in [0, sc->max_devices) where
sc->max_devices = facts->MaxTargets + facts->MaxVolumes (mps_mapping.c:1734),
and IR volume entries occupy the top max_volumes slots in HIGH volume-mapping
mode (mps_mapping.c:232-234).  sassc->targets was allocated with only
facts->MaxTargets entries (mps_sas.c:696-697), so every
`sassc->targets[id]` with a volume ID (mpssas_volume_add, IR-config REMOVED
branch in mps_sas_lsi.c:847/:310) writes past the end of the allocation.
mpssas_find_target_by_handle also stopped at MaxTargets, making volume
targets invisible to the "already added" check and to removal handling.

Not applied to sys/ (read-only audit tree); authored after verification.

--- a/sys/dev/raid/mps/mps_sas.h
+++ b/sys/dev/raid/mps/mps_sas.h
@@ -95,6 +95,7 @@ struct mpssas_softc {
 	struct mpssas_target	*targets;
+	u_int			maxtargets;	/* entries in targets[] */
 	struct cam_devq		*devq;
--- a/sys/dev/raid/mps/mps_sas.c
+++ b/sys/dev/raid/mps/mps_sas.c
@@ -178,7 +178,7 @@ mpssas_find_target_by_handle(struct mpssas_softc *sassc, int start,
 	struct mpssas_target *target;
 	int i;
 
-	for (i = start; i < sassc->sc->facts->MaxTargets; i++) {
+	for (i = start; i < sassc->maxtargets; i++) {
 		target = &sassc->targets[i];
 		if (target->handle == handle)
 			return (target);
@@ -693,8 +693,17 @@ mps_attach_sas(struct mps_softc *sc)
 	sassc = kmalloc(sizeof(struct mpssas_softc), M_MPT2, M_WAITOK|M_ZERO);
-	sassc->targets = kmalloc(sizeof(struct mpssas_target) *
-	    sc->facts->MaxTargets, M_MPT2, M_WAITOK|M_ZERO);
+	/*
+	 * The mapping layer (mps_mapping.c: mps_mapping_initialize,
+	 * _mapping_get_ir_maprange) hands out target IDs in
+	 * [0, facts->MaxTargets + facts->MaxVolumes): with HIGH volume
+	 * mapping the IR volume IDs are exactly
+	 * [MaxTargets, MaxTargets+MaxVolumes-1].  Those IDs are used to
+	 * index sassc->targets[] (mpssas_volume_add,
+	 * mps_sas_lsi.c:847), so the array must cover the whole ID space.
+	 */
+	sassc->maxtargets = sc->facts->MaxTargets + sc->facts->MaxVolumes;
+	sassc->targets = kmalloc(sizeof(struct mpssas_target) *
+	    sassc->maxtargets, M_MPT2, M_WAITOK|M_ZERO);
 	sc->sassc = sassc;

Validated with the harness fixed-model arm (voladd-HIGH-FIXED): the exact
same HIGH-mapping volume-registration write sequence stays in bounds and
the guarded run exits clean, while the stock sizing SIGSEGVs (run.log).