⬢ DragonFlyBSD Kernel Audit
DF-2204 / 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
diff --git a/sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c b/sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c
index 0000000..1111111 100644
--- a/sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/sys/dev/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -82,6 +82,22 @@
 	if (!hive)
 		goto exit;
 
+	/*
+	 * DF-2204 (part 1): amdgpu_xgmi_add_device is not idempotent.
+	 * Without this check, a second call for an adev whose node is already
+	 * linked would list_add_tail() it again, corrupting the
+	 * doubly-linked list (loops, visits to freed memory).  amdgpu_device
+	 * re-init paths (driver reset / rebind) re-enter this function.
+	 */
+	if (!list_empty(&adev->gmc.xgmi.head)) {
+		dev_warn(adev->dev,
+			"XGMI: node %llx already in hive %llx; not re-adding",
+			adev->gmc.xgmi.device_id,
+			adev->gmc.xgmi.hive_id);
+		ret = 0;
+		goto exit;
+	}
+
 	list_add_tail(&adev->gmc.xgmi.head, &hive->device_list);
 	list_for_each_entry(entry, &hive->device_list, head)
 		tmp_topology[count++].device_id = entry->device_id;
@@ -116,4 +132,28 @@
 	return ret;
 }
 
+/*
+ * DF-2204 (part 2): The tree had no amdgpu_xgmi_remove_device() and no
+ * list_del() of xgmi.head anywhere, so once a device was list_add_tail()ed
+ * into a hive (line above) it stayed linked forever -- even after the
+ * amdgpu_device was torn down and freed.  Any later amdgpu_xgmi_add_device()
+ * for any device in the same hive then iterated the freed node (UAF).
+ *
+ * amdgpu_device_fini() now calls this before freeing the device so the node
+ * is unlinked while adev is still valid.
+ */
+void amdgpu_xgmi_remove_device(struct amdgpu_device *adev)
+{
+	if ((adev->asic_type < CHIP_VEGA20) ||
+		(adev->flags & AMD_IS_APU))
+		return;
+
+	mutex_lock(&xgmi_mutex);
+	if (!list_empty(&adev->gmc.xgmi.head)) {
+		list_del(&adev->gmc.xgmi.head);
+		INIT_LIST_HEAD(&adev->gmc.xgmi.head);
+	}
+	mutex_unlock(&xgmi_mutex);
+}
+
 

diff --git a/sys/dev/drm/amd/amdgpu/amdgpu.h b/sys/dev/drm/amd/amdgpu/amdgpu.h
index 0000000..2222222 100644
--- a/sys/dev/drm/amd/amdgpu/amdgpu.h
+++ b/sys/dev/drm/amd/amdgpu/amdgpu.h
@@ -1250,6 +1250,7 @@
  * functions used by amdgpu_xgmi.c
  */
 int amdgpu_xgmi_add_device(struct amdgpu_device *adev);
+void amdgpu_xgmi_remove_device(struct amdgpu_device *adev);
 
 /*
  * functions used by amdgpu_encoder.c

diff --git a/sys/dev/drm/amd/amdgpu/amdgpu_device.c b/sys/dev/drm/amd/amdgpu/amdgpu_device.c
index 0000000..3333333 100644
--- a/sys/dev/drm/amd/amdgpu/amdgpu_device.c
+++ b/sys/dev/drm/amd/amdgpu/amdgpu_device.c
@@ -2730,6 +2730,8 @@
 
 	DRM_INFO("amdgpu: finishing device.\n");
 	adev->shutdown = true;
+	/* DF-2204: unlink from XGMI hive while adev is still valid */
+	amdgpu_xgmi_remove_device(adev);
 	/* disable all interrupts */
 	amdgpu_irq_disable_all(adev);
 	if (adev->mode_info.mode_config_initialized){