drm/amd/powerplay: Fix buffer overflows with mc_reg_address
authorErnst Sjöstrand <ernstp@gmail.com>
Sun, 19 Nov 2017 17:52:45 +0000 (18:52 +0100)
committerAlex Deucher <alexander.deucher@amd.com>
Wed, 6 Dec 2017 17:48:00 +0000 (12:48 -0500)
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.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Ernst Sjöstrand <ernstp@gmail.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
drivers/gpu/drm/amd/amdgpu/ci_dpm.c
drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c

index 5a60c161b0fc1cf775d0963d6218c11564cd5146..f11c0aacf19f5a34652da90522a6d80264c09cc0 100644 (file)
@@ -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;
index 0a8e48bff219e2f3187e1958fb0c78b217d32d36..81b8790c0d2210740a196799bb64108718af6d97 100644 (file)
@@ -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: