⬢ DragonFlyBSD Kernel Audit
DF-4059 / 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
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
# DF-4059 + DF-4060 -- sys/dev/raid/amr/amr.c
#
# DF-4059: amr_done() trusts the controller completion list twice over.
#   (a) completion id 0 yields idx = -1 and reads amr_busycmd[-1] -- the
#       last field of amr_ready (always &amr_ready.stqh_first, or
#       &requeued_cmd.ac_link after an EBUSY requeue) -- which is then
#       driven through amr_freeslot() as an amr_command*: with the queue
#       idle this lands on a NULL slot pointer and panics ("amr: slot 0
#       not busy?"); with a requeued command it double-frees a live
#       command's slot; with slot 0 busy the forged ac_sc/ac_slot reads
#       steer a NULL store and an atomic decrement into arbitrary softc
#       fields and past the end of the softc (verified 1208 bytes past
#       the 3920-byte softc on the audit rig).  id 0 is never allocated
#       (ident = slot + 1, amr.c:2038/2074), so reject it outright.
#   (b) on std firmware mb_nstatus is an 8-bit controller-chosen count
#       but mb_completed[] has 46 entries: the loop reads up to 255
#       entries -- 209 bytes past the 82-byte stack mailbox -- and every
#       out-of-bounds byte is disclosed through the "bad slot %d
#       completed" print (verified: kernel heap pointers and caller
#       frame contents disclosed to the console log).  Clamp the loop to
#       the array size.  (The quartz-side WRITE overflow into
#       completed[46] inside amr_quartz_get_work is DF-1358 and is fixed
#       separately.)
#
# DF-4060: amr_std_poll_command() timeout leaves the slot permanently
#   busy (busycmd/busyslots/AMR_CMD_BUSY) while every caller
#   (amr_enquiry, amr_flush, amr_support_ext_cdb, amr_dump_blocks)
#   unconditionally amr_releasecmd()s the command.  The next
#   amr_alloccmd/amr_start panics ("amr: slot %d busy?"), a late
#   completion completes the recycled command while it sits on the free
#   list (status store + completion callback into a free-listed command,
#   verified), and amr_rescan_drives spins forever on the poisoned
#   busyslots counter.  Detach the slot on timeout so late completions
--- a/sys/dev/raid/amr/amr.c	2026-10-15 11:53:50.510749327 +0000
+++ b/sys/dev/raid/amr/amr.c	2026-10-15 11:53:50.522749173 +0000
@@ -1356,8 +1356,21 @@
     if (!(ac->ac_flags & AMR_CMD_BUSY)) {
 	error = 0;
     } else {
-	/* XXX the slot is now marked permanently busy */
+	/*
+	 * Timed out.  Detach the command from its slot so that the
+	 * caller's amr_releasecmd() cannot recycle a command the
+	 * controller may still complete late: a late completion will
+	 * now be rejected as a bad slot instead of completing the
+	 * recycled command.
+	 */
 	error = EIO;
+	lockmgr(&sc->amr_hw_lock, LK_EXCLUSIVE);
+	if (sc->amr_busycmd[ac->ac_slot] == ac) {
+	    sc->amr_busycmd[ac->ac_slot] = NULL;
+	    atomic_subtract_int(&sc->amr_busyslots, 1);
+	}
+	lockmgr(&sc->amr_hw_lock, LK_RELEASE);
+	ac->ac_flags &= ~AMR_CMD_BUSY;
 	device_printf(sc->amr_dev, "polled command timeout\n");
     }
     return(error);
@@ -1768,7 +1781,7 @@
     ac_qhead_t		head;
     struct amr_command	*ac;
     struct amr_mailbox	mbox;
-    int			i, idx, result;
+    int			i, idx, nstatus, result;
     
     debug_called(3);
 
@@ -1781,9 +1794,27 @@
 	/* poll for a completed command's identifier and status */
 	if (sc->amr_get_work(sc, &mbox)) {
 	    result = 1;
-	    
+
+	    /*
+	     * The completion list has a fixed size.  A larger count from
+	     * the controller (mb_nstatus is 8 bits, the array holds 46)
+	     * would read past the mailbox on our stack.
+	     */
+	    nstatus = mbox.mb_nstatus;
+	    if (nstatus > (int)nitems(mbox.mb_completed)) {
+		device_printf(sc->amr_dev, "excessive completion count %d\n",
+			      nstatus);
+		nstatus = nitems(mbox.mb_completed);
+	    }
+
 	    /* iterate over completed commands in this result */
-	    for (i = 0; i < mbox.mb_nstatus; i++) {
+	    for (i = 0; i < nstatus; i++) {
+		/* completion id 0 is never allocated (ident = slot + 1) */
+		if (mbox.mb_completed[i] < 1) {
+		    device_printf(sc->amr_dev, "bad completion id %d\n",
+				      mbox.mb_completed[i]);
+		    continue;
+		}
 		/* get pointer to busy command */
 		idx = mbox.mb_completed[i] - 1;
 		ac = sc->amr_busycmd[idx];