⬢ DragonFlyBSD Kernel Audit
DF-4560 / fix.diff
← back to finding ↓ download raw 
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
fix.diff — DF-4560: validate that EventDataLength covers the structure before
the fixed-offset member reads in mpssas_fw_work (mps_sas_lsi.c).

mpssas_evt_handler sizes the event copy as sz = EventDataLength*4
(mps_sas_lsi.c:140-148) but the handlers dereference fixed struct members
without checking that sz covers them:
  - IR CONFIG CHANGE LIST reads element->VolDevHandle at +0x0A
    (mps_sas_lsi.c:276-279) unconditionally, even when NumElements == 0;
  - LOG ENTRY ADDED reads LogEntryQualifier at +0x0E and LogData[0] at
    +0x14 (mps_sas_lsi.c:515-518).

Fix: carry sz in mps_fw_event_work and gate both sinks on it.  (This
complements, not replaces, DF-1696 (sz computation) and DF-1697 (loop
counts); those fixes do not cover these two sinks.)

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

--- a/sys/dev/raid/mps/mps_sas_lsi.c
+++ b/sys/dev/raid/mps/mps_sas_lsi.c
@@ -83,6 +83,7 @@ struct mps_fw_event_work {
 	u16			event;
+	u16			sz;		/* size of event_data, bytes */
 	void			*event_data;
 	TAILQ_ENTRY(mps_fw_event_work)	ev_link;
@@ -149,6 +150,7 @@ mpssas_evt_handler(struct mps_softc *sc, uintptr_t data,
 	bcopy(event->EventData, fw_event->event_data, sz);
+	fw_event->sz = sz;
 	fw_event->event = event->Event;
@@ -272,9 +274,14 @@ mpssas_fw_work(struct mps_softc *sc, struct mps_fw_event_work *fw_event)
 		unsigned int id;
 
 		event_data = fw_event->event_data;
+		if (fw_event->sz < sizeof(*event_data)) {
+			kprintf("%s: short IR config change event (%u bytes)\n",
+			    __func__, fw_event->sz);
+			break;
+		}
 		foreign_config = (le32toh(event_data->Flags) &
 		    MPI2_EVENT_IR_CHANGE_FLAGS_FOREIGN_CONFIG) ? 1 : 0;
 
 		element =
 		    (Mpi2EventIrConfigElement_t *)&event_data->ConfigElement[0];
-		id = mps_mapping_get_raid_id_from_handle
-		    (sc, element->VolDevHandle);
+		id = MPS_MAP_BAD_ID;
+		if (event_data->NumElements > 0 &&
+		    fw_event->sz >= sizeof(*event_data) +
+		    sizeof(Mpi2EventIrConfigElement_t))
+			id = mps_mapping_get_raid_id_from_handle
+			    (sc, element->VolDevHandle);
@@ -512,6 +519,11 @@ mpssas_fw_work(struct mps_softc *sc, struct mps_fw_event_work *fw_event)
 		uint8_t				logCode;
 
 		logEntry = (pMpi2EventDataLogEntryAdded_t)fw_event->event_data;
+		if (fw_event->sz < offsetof(Mpi2EventDataLogEntryAdded_t,
+					     LogData) + 1) {
+			kprintf("%s: short log entry event (%u bytes)\n",
+			    __func__, fw_event->sz);
+			break;
+		}
 		logQualifier = logEntry->LogEntryQualifier;