[PATCH 4/5] drm/amd/powerplay: Fix buffer overflows with mc_reg_address

Ernst Sjöstrand ernstp at gmail.com
Sun Nov 19 17:52:45 UTC 2017


Smatch warned about the following lines:
ci_set_mc_special_registers() error: buffer overflow 'table->mc_reg_address' 16 <= 16
tonga_set_mc_special_registers() error: buffer overflow 'table->mc_reg_address' 16 <= 16

Change the logic to check before access instead of after incrementing.
It's fine if j reaches max after we're done. This allows the last entry
of the array to be filled without an error message for example.
Changed some whitespace to clarify grouping.

Signed-off-by: Ernst Sjöstrand <ernstp at gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/ci_dpm.c                 | 10 +++-------
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c | 10 +++-------
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/ci_dpm.c b/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
index 5a60c161b0fc..f11c0aacf19f 100644
--- a/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
+++ b/drivers/gpu/drm/amd/amdgpu/ci_dpm.c
@@ -4540,9 +4540,9 @@ static int ci_set_mc_special_registers(struct amdgpu_device *adev,
 					((temp_reg & 0xffff0000)) | ((table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16);
 			}
 			j++;
+
 			if (j >= SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
 				return -EINVAL;
-
 			temp_reg = RREG32(mmMC_PMG_CMD_MRS);
 			table->mc_reg_address[j].s1 = mmMC_PMG_CMD_MRS;
 			table->mc_reg_address[j].s0 = mmMC_SEQ_PMG_CMD_MRS_LP;
@@ -4553,10 +4553,10 @@ static int ci_set_mc_special_registers(struct amdgpu_device *adev,
 					table->mc_reg_table_entry[k].mc_data[j] |= 0x100;
 			}
 			j++;
-			if (j > SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
-				return -EINVAL;
 
 			if (adev->mc.vram_type != AMDGPU_VRAM_TYPE_GDDR5) {
+				if (j >= SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
+					return -EINVAL;
 				table->mc_reg_address[j].s1 = mmMC_PMG_AUTO_CMD;
 				table->mc_reg_address[j].s0 = mmMC_PMG_AUTO_CMD;
 				for (k = 0; k < table->num_entries; k++) {
@@ -4564,8 +4564,6 @@ static int ci_set_mc_special_registers(struct amdgpu_device *adev,
 						(table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16;
 				}
 				j++;
-				if (j > SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
-					return -EINVAL;
 			}
 			break;
 		case mmMC_SEQ_RESERVE_M:
@@ -4577,8 +4575,6 @@ static int ci_set_mc_special_registers(struct amdgpu_device *adev,
 					(temp_reg & 0xffff0000) | (table->mc_reg_table_entry[k].mc_data[i] & 0x0000ffff);
 			}
 			j++;
-			if (j > SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
-				return -EINVAL;
 			break;
 		default:
 			break;
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c
index 0a8e48bff219..81b8790c0d22 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c
@@ -3106,9 +3106,9 @@ static int tonga_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 					((table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16);
 			}
 			j++;
+
 			PP_ASSERT_WITH_CODE((j < SMU72_DISCRETE_MC_REGISTER_ARRAY_SIZE),
 				"Invalid VramInfo table.", return -EINVAL);
-
 			temp_reg = cgs_read_register(hwmgr->device, mmMC_PMG_CMD_MRS);
 			table->mc_reg_address[j].s1 = mmMC_PMG_CMD_MRS;
 			table->mc_reg_address[j].s0 = mmMC_SEQ_PMG_CMD_MRS_LP;
@@ -3121,18 +3121,16 @@ static int tonga_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 					table->mc_reg_table_entry[k].mc_data[j] |= 0x100;
 			}
 			j++;
-			PP_ASSERT_WITH_CODE((j <= SMU72_DISCRETE_MC_REGISTER_ARRAY_SIZE),
-				"Invalid VramInfo table.", return -EINVAL);
 
 			if (!data->is_memory_gddr5) {
+				PP_ASSERT_WITH_CODE((j < SMU72_DISCRETE_MC_REGISTER_ARRAY_SIZE),
+					"Invalid VramInfo table.", return -EINVAL);
 				table->mc_reg_address[j].s1 = mmMC_PMG_AUTO_CMD;
 				table->mc_reg_address[j].s0 = mmMC_PMG_AUTO_CMD;
 				for (k = 0; k < table->num_entries; k++)
 					table->mc_reg_table_entry[k].mc_data[j] =
 						(table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16;
 				j++;
-				PP_ASSERT_WITH_CODE((j <= SMU72_DISCRETE_MC_REGISTER_ARRAY_SIZE),
-					"Invalid VramInfo table.", return -EINVAL);
 			}
 
 			break;
@@ -3147,8 +3145,6 @@ static int tonga_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 					(table->mc_reg_table_entry[k].mc_data[i] & 0x0000ffff);
 			}
 			j++;
-			PP_ASSERT_WITH_CODE((j <= SMU72_DISCRETE_MC_REGISTER_ARRAY_SIZE),
-				"Invalid VramInfo table.", return -EINVAL);
 			break;
 
 		default:
-- 
2.14.1



More information about the amd-gfx mailing list