⬢ DragonFlyBSD Kernel Audit
DF-4476 / 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
--- a/sys/dev/disk/nata/ata-all.c
+++ b/sys/dev/disk/nata/ata-all.c
@@ -83,6 +83,7 @@
 int ata_wc = 1;
 
 /* local vars */
+static struct lock ata_channel_ctl_lock;	/* DF-4476: serialize channel control ioctls */
 static int ata_dma = 1;
 static int atapi_dma = 1;
 
@@ -385,8 +386,10 @@
 	if (*value >= devclass_get_maxunit(ata_devclass) ||
 	    !(device = devclass_get_device(ata_devclass, *value)))
 	    return ENXIO;
+	lockmgr(&ata_channel_ctl_lock, LK_EXCLUSIVE);
 	error = ata_reinit(device);
 	ata_start(device);
+	lockmgr(&ata_channel_ctl_lock, LK_RELEASE);
 	break;
 
     case IOCATAATTACH:
@@ -394,14 +397,18 @@
 	    !(device = devclass_get_device(ata_devclass, *value)))
 	    return ENXIO;
 	/* XXX SOS should enable channel HW on controller */
+	lockmgr(&ata_channel_ctl_lock, LK_EXCLUSIVE);
 	error = ata_attach(device);
+	lockmgr(&ata_channel_ctl_lock, LK_RELEASE);
 	break;
 
     case IOCATADETACH:
 	if (*value >= devclass_get_maxunit(ata_devclass) ||
 	    !(device = devclass_get_device(ata_devclass, *value)))
 	    return ENXIO;
+	lockmgr(&ata_channel_ctl_lock, LK_EXCLUSIVE);
 	error = ata_detach(device);
+	lockmgr(&ata_channel_ctl_lock, LK_RELEASE);
 	/* XXX SOS should disable channel HW on controller */
 	break;
 
@@ -485,7 +492,17 @@
 	    request->u.ata.lba = ioc_request->u.ata.lba;
 	    request->u.ata.count = ioc_request->u.ata.count;
 	}
-	request->timeout = ioc_request->timeout;
+	/*
+	 * DF-4475: clamp the untrusted timeout.  A negative timeout makes
+	 * the ata-queue completion wait (timeout * hz * 4) wrap to a ~1-tick
+	 * expiry, so a request merely queued behind others hits the
+	 * "taskqueue timeout" fallback and is then freed by this ioctl
+	 * while the channel still owns it (use-after-free).
+	 */
+	if (ioc_request->timeout <= 0)
+	    request->timeout = ATA_DEFAULT_TIMEOUT;
+	else
+	    request->timeout = ioc_request->timeout;
 	request->data = buf;
 	request->bytecount = ioc_request->count;
 	request->transfersize = request->bytecount;
@@ -1077,6 +1094,7 @@
 					objcache_malloc_alloc,
 					objcache_malloc_free,
 					&ata_request_malloc_args);
+    lockinit(&ata_channel_ctl_lock, "atactl", 0, 0);
     ata_composite_cache = objcache_create("ata_composite", 0, 0,
 					  ata_composite_cache_ctor, NULL, NULL,
 					  objcache_malloc_alloc,