⬢ DragonFlyBSD Kernel Audit
DF-4187 / fix_combined.diff
← back to finding ↓ download raw
--- a/sys/dev/drm/radeon/sumo_dpm.c
+++ b/sys/dev/drm/radeon/sumo_dpm.c
@@ -1206,6 +1206,15 @@
 {
 	struct sumo_power_info *pi = sumo_get_pi(rdev);
 
+	/* DF-4187: the VBIOS power table may contain no BOOT-classified
+	 * state (or dpm.init may have failed and radeon_pm swallowed it),
+	 * leaving boot_ps NULL.  Refuse to enable instead of page-faulting
+	 * in sumo_update_current_ps(rdev, NULL) below. */
+	if (rdev->pm.dpm.boot_ps == NULL) {
+		DRM_ERROR("no boot power state, refusing to enable DPM\n");
+		return -EINVAL;
+	}
+
 	if (sumo_dpm_enabled(rdev))
 		return -EINVAL;
 
@@ -1263,6 +1272,8 @@
 
 	if (!sumo_dpm_enabled(rdev))
 		return;
+	if (rdev->pm.dpm.boot_ps == NULL)	/* DF-4187 */
+		return;
 	sumo_disable_clock_power_gating(rdev);
 	if (pi->enable_sclk_ds)
 		sumo_enable_sclk_ds(rdev, false);
@@ -1493,13 +1504,31 @@
 		u8 *idx;
 		power_state = (union pplib_power_state *)power_state_offset;
 		non_clock_array_index = power_state->v2.nonClockInfoIndex;
+		/* DF-4190: both indices are u8 values from the VBIOS image and
+		 * were never bounds-checked; a crafted table reads past the
+		 * NonClockInfoArray / ClockInfoArray in the kmalloc'd BIOS
+		 * image. */
+		if (non_clock_array_index >= non_clock_info_array->ucNumEntries) {
+			DRM_ERROR("invalid nonClockInfoIndex %d\n",
+			    non_clock_array_index);
+			kfree(rdev->pm.dpm.ps);
+			rdev->pm.dpm.ps = NULL;
+			return -EINVAL;
+		}
 		non_clock_info = (struct _ATOM_PPLIB_NONCLOCK_INFO *)
 			&non_clock_info_array->nonClockInfo[non_clock_array_index];
 		if (!rdev->pm.power_state[i].clock_info)
 			return -EINVAL;
 		ps = kzalloc(sizeof(struct sumo_ps), GFP_KERNEL);
 		if (ps == NULL) {
+			/* DF-4188: free the already-parsed ps_priv states and
+			 * NULL the array pointer; leaving it dangling makes
+			 * sumo_dpm_fini() double-free it after the (swallowed,
+			 * see radeon_pm.c DF-4162) init failure. */
+			for (i--; i >= 0; i--)
+				kfree(rdev->pm.dpm.ps[i].ps_priv);
 			kfree(rdev->pm.dpm.ps);
+			rdev->pm.dpm.ps = NULL;
 			return -ENOMEM;
 		}
 		rdev->pm.dpm.ps[i].ps_priv = ps;
@@ -1509,6 +1538,15 @@
 			clock_array_index = idx[j];
 			if (k >= SUMO_MAX_HARDWARE_POWERLEVELS)
 				break;
+			if (clock_array_index >= clock_info_array->ucNumEntries) {
+				DRM_ERROR("invalid clockInfoIndex %d\n",
+				    clock_array_index);
+				for (; i >= 0; i--)
+					kfree(rdev->pm.dpm.ps[i].ps_priv);
+				kfree(rdev->pm.dpm.ps);
+				rdev->pm.dpm.ps = NULL;
+				return -EINVAL;
+			}
 
 			clock_info = (union pplib_clock_info *)
 				((u8 *)&clock_info_array->clockInfo[0] +
@@ -1518,6 +1556,21 @@
 						    clock_info);
 			k++;
 		}
+		/* DF-4189: a pplib state with ucNumDPMLevels == 0 would leave
+		 * ps->num_levels == 0; every later levels[num_levels - 1]
+		 * access (sumo_set_uvd_clock_*_set_eng_clock, sumo_program_bsp,
+		 * sumo_dpm_get_sclk) then reads levels[0xffffffff], ~96 GB
+		 * past the allocation.  Reject the table instead. */
+		if (k == 0) {
+			DRM_ERROR("power state %d has no DPM levels\n", i);
+			kfree(ps);
+			rdev->pm.dpm.ps[i].ps_priv = NULL;
+			for (i--; i >= 0; i--)
+				kfree(rdev->pm.dpm.ps[i].ps_priv);
+			kfree(rdev->pm.dpm.ps);
+			rdev->pm.dpm.ps = NULL;
+			return -EINVAL;
+		}
 		sumo_parse_pplib_non_clock_info(rdev, &rdev->pm.dpm.ps[i],
 						non_clock_info,
 						non_clock_info_array->ucEntrySize);