[PATCH] drm/radeon: fix potential buffer overflow in ni_set_mc_special_registers()

[Alex Deucher]

Applied.  Thanks!

Alex

On Mon, Jun 6, 2022 at 9:51 AM Alexey Kodanev
<aleksei.kodanev at bell-sw.com> 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 at bell-sw.com>
> ---
>  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
>