Applied. Thanks!
Alex
On Mon, Jun 6, 2022 at 9:51 AM Alexey Kodanev
<aleksei.kodanev@xxxxxxxxxxx> wrote:
>
> The last case label can write two buffers 'mc_reg_address[j]' and
> 'mc_data[j]' with 'j' offset equal to SMC_NISLANDS_MC_REGISTER_ARRAY_SIZE
> since there are no checks for this value in both case labels after the
> last 'j++'.
>
> Instead of changing '>' to '>=' there, add the bounds check at the start
> of the second 'case' (the first one already has it).
>
> Also, remove redundant last checks for 'j' index bigger than array size.
> The expression is always false. Moreover, before or after the patch
> 'table->last' can be equal to SMC_NISLANDS_MC_REGISTER_ARRAY_SIZE and it
> seems it can be a valid value.
>
> Detected using the static analysis tool - Svace.
> Fixes: 69e0b57a91ad ("drm/radeon/kms: add dpm support for cayman (v5)")
> Signed-off-by: Alexey Kodanev <aleksei.kodanev@xxxxxxxxxxx>
> ---
> drivers/gpu/drm/radeon/ni_dpm.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/ni_dpm.c b/drivers/gpu/drm/radeon/ni_dpm.c
> index 769f666335ac..672d2239293e 100644
> --- a/drivers/gpu/drm/radeon/ni_dpm.c
> +++ b/drivers/gpu/drm/radeon/ni_dpm.c
> @@ -2741,10 +2741,10 @@ static int ni_set_mc_special_registers(struct radeon_device *rdev,
> table->mc_reg_table_entry[k].mc_data[j] |= 0x100;
> }
> j++;
> - if (j > SMC_NISLANDS_MC_REGISTER_ARRAY_SIZE)
> - return -EINVAL;
> break;
> case MC_SEQ_RESERVE_M >> 2:
> + if (j >= SMC_NISLANDS_MC_REGISTER_ARRAY_SIZE)
> + return -EINVAL;
> temp_reg = RREG32(MC_PMG_CMD_MRS1);
> table->mc_reg_address[j].s1 = MC_PMG_CMD_MRS1 >> 2;
> table->mc_reg_address[j].s0 = MC_SEQ_PMG_CMD_MRS1_LP >> 2;
> @@ -2753,8 +2753,6 @@ static int ni_set_mc_special_registers(struct radeon_device *rdev,
> (temp_reg & 0xffff0000) |
> (table->mc_reg_table_entry[k].mc_data[i] & 0x0000ffff);
> j++;
> - if (j > SMC_NISLANDS_MC_REGISTER_ARRAY_SIZE)
> - return -EINVAL;
> break;
> default:
> break;
> --
> 2.25.1
>
