[PATCH v1 2/2] amdgpu/pm: Fix possible array out-of-bounds if SCLK levels != 2

Darren Powell darren.powell at amd.com
Mon May 9 03:58:38 UTC 2022


 added a check to populate and use SCLK shim table freq_values only
   if using dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL or
                         AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM
 removed clocks.num_levels from calculation of shim table size
 removed unsafe accesses to shim table freq_values
   output gfx_table values if using other dpm levels
 added check for freq_match when using freq_values for when now == min_clk

== Test ==
LOGFILE=aldebaran-sclk.test.log
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1`
AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | awk '{print $9}'`
HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}

lspci -nn | grep "VGA\|Display"  > $LOGFILE
FILES="pp_od_clk_voltage
pp_dpm_sclk"

for f in $FILES
do
  echo === $f === >> $LOGFILE
  cat $HWMON_DIR/device/$f >> $LOGFILE
done
cat $LOGFILE

Signed-off-by: Darren Powell <darren.powell at amd.com>
---
 .../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c    | 60 +++++++++----------
 1 file changed, 29 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
index 6a4fca47ae53..a653668e8402 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
@@ -740,9 +740,8 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
 	struct smu_13_0_dpm_table *single_dpm_table;
 	struct smu_dpm_context *smu_dpm = &smu->smu_dpm;
 	struct smu_13_0_dpm_context *dpm_context = NULL;
-	uint32_t display_levels;
 	uint32_t freq_values[3] = {0};
-	uint32_t min_clk, max_clk;
+	uint32_t min_clk, max_clk, display_levels = 0;
 
 	smu_cmn_get_sysfs_buf(&buf, &size);
 
@@ -765,46 +764,45 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,
 			return ret;
 		}
 
-		single_dpm_table = &(dpm_context->dpm_tables.gfx_table);
-		ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
-		if (ret) {
-			dev_err(smu->adev->dev, "Attempt to get gfx clk levels Failed!");
-			return ret;
-		}
-
-		display_levels = clocks.num_levels;
-
-		min_clk = pstate_table->gfxclk_pstate.curr.min;
-		max_clk = pstate_table->gfxclk_pstate.curr.max;
-
-		freq_values[0] = min_clk;
-		freq_values[1] = max_clk;
-
-		/* fine-grained dpm has only 2 levels */
-		if (now > min_clk && now < max_clk) {
-			display_levels = clocks.num_levels + 1;
-			freq_values[2] = max_clk;
-			freq_values[1] = now;
-		}
+		if ((smu_dpm->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL &&
+		     smu_dpm->dpm_level != AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM)) {
+			single_dpm_table = &(dpm_context->dpm_tables.gfx_table);
+			ret = aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
+			if (ret) {
+				dev_err(smu->adev->dev, "Attempt to get gfx clk levels Failed!");
+				return ret;
+			}
 
-		/*
-		 * For DPM disabled case, there will be only one clock level.
-		 * And it's safe to assume that is always the current clock.
-		 */
-		if (display_levels == clocks.num_levels) {
 			for (i = 0; i < clocks.num_levels; i++)
 				size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,
-					freq_values[i],
+					clocks.data[i].clocks_in_khz / 1000,
 					(clocks.num_levels == 1) ?
 						"*" :
 						(aldebaran_freqs_in_same_level(
-							 freq_values[i], now) ?
+							 clocks.data[i].clocks_in_khz / 1000, now) ?
 							 "*" :
 							 ""));
 		} else {
+			/* fine-grained dpm has only 2 levels */
+			display_levels = 2;
+
+			min_clk = pstate_table->gfxclk_pstate.curr.min;
+			max_clk = pstate_table->gfxclk_pstate.curr.max;
+
+			freq_values[0] = min_clk;
+			freq_values[1] = max_clk;
+
+			if (now > min_clk && now < max_clk) {
+				display_levels++;
+				freq_values[2] = max_clk;
+				freq_values[1] = now;
+			}
+
 			for (i = 0; i < display_levels; i++)
 				size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,
-						freq_values[i], i == 1 ? "*" : "");
+						freq_values[i],
+						aldebaran_freqs_in_same_level(freq_values[i], now) ?
+							"*" : "");
 		}
 
 		break;
-- 
2.35.1



More information about the amd-gfx mailing list