⬢ DragonFlyBSD Kernel Audit
DF-1359 / 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
62
63
64
65
66
67
68
69
diff --git a/sys/dev/raid/mps/mps_user.c b/sys/dev/raid/mps/mps_user.c
--- a/sys/dev/raid/mps/mps_user.c
+++ b/sys/dev/raid/mps/mps_user.c
@@ -798,15 +798,24 @@
 	/*
 	 * copy in the header so we know what we're dealing with before we
 	 * commit to allocating a command for it.
+	 *
+	 * DF-1359: tmphdr is a 12-byte MPI2_REQUEST_HEADER on this stack
+	 * frame.  An unchecked copyin of the user-controlled RequestSize
+	 * overflows tmphdr (write past stack).  The later bcopy of
+	 * &tmphdr with the same RequestSize at the task/hdr sites also
+	 * reads past tmphdr (read past stack).  Bounds-check RequestSize
+	 * BEFORE the copyin and only copy in the header bytes we will
+	 * actually peek at here; the full request is re-read directly from
+	 * userspace into the properly-sized cm->cm_req at the bcopy sites.
 	 */
-	err = copyin(PTRIN(data->PtrRequest), &tmphdr, data->RequestSize);
-	if (err != 0)
-		goto RetFreeUnlocked;
-
-	if (data->RequestSize > (int)sc->facts->IOCRequestFrameSize * 4) {
+	if (data->RequestSize < (int)sizeof(tmphdr) ||
+	    data->RequestSize > (int)sc->facts->IOCRequestFrameSize * 4) {
 		err = EINVAL;
 		goto RetFreeUnlocked;
 	}
+	err = copyin(PTRIN(data->PtrRequest), &tmphdr, sizeof(tmphdr));
+	if (err != 0)
+		goto RetFreeUnlocked;
 
 	function = tmphdr.Function;
 	mps_dprint(sc, MPS_INFO, "%s: Function %02X MsgFlags %02X\n", __func__,
@@ -825,9 +834,17 @@
 			goto Ret;
 		}
 
-		/* Copy the header in.  Only a small fixup is needed. */
+		/* DF-1359: Copy the full request directly from userspace
+		 * into the properly-sized HW command buffer.  Reading from
+		 * &tmphdr with data->RequestSize would read past the 12-byte
+		 * stack header. */
 		task = (MPI2_SCSI_TASK_MANAGE_REQUEST *)cm->cm_req;
-		bcopy(&tmphdr, task, data->RequestSize);
+		err = copyin(PTRIN(data->PtrRequest), task, data->RequestSize);
+		if (err != 0) {
+			mps_dprint(sc, MPS_FAULT, "%s: copyin task failed",
+			    __func__);
+			goto Ret;
+		}
 		task->TaskMID = cm->cm_desc.Default.SMID;
 
 		cm->cm_data = NULL;
@@ -875,7 +892,14 @@
 	mps_unlock(sc);
 
 	hdr = (MPI2_REQUEST_HEADER *)cm->cm_req;
-	bcopy(&tmphdr, hdr, data->RequestSize);
+	/* DF-1359: read the full request directly from userspace; the old
+	 * bcopy(&tmphdr, hdr, RequestSize) read past the 12-byte stack
+	 * header. */
+	err = copyin(PTRIN(data->PtrRequest), hdr, data->RequestSize);
+	if (err != 0) {
+		mps_dprint(sc, MPS_FAULT, "%s: copyin hdr failed", __func__);
+		goto Ret;
+	}
 
 	/*
 	 * Do some checking to make sure the IOCTL request contains a valid