[PATCH v3] drm/amdgpu: Fix potential out-of-bounds access in 'amdgpu_discovery_reg_base_init()'

[Alex Deucher]

On Fri, Feb 2, 2024 at 10:53 AM Srinivasan Shanmugam
<srinivasan.shanmugam at amd.com> wrote:
>
> The issue arises when the array 'adev->vcn.vcn_config' is accessed
> before checking if the index 'adev->vcn.num_vcn_inst' is within the
> bounds of the array.
>
> The fix involves moving the bounds check before the array access. This
> ensures that 'adev->vcn.num_vcn_inst' is within the bounds of the array
> before it is used as an index.
>
> Fixes the below:
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1289 amdgpu_discovery_reg_base_init() error: testing array offset 'adev->vcn.num_vcn_inst' after use.
>
> Fixes: aaf1090a6cb6 ("drm/amdgpu: Add instance mask for VCN and JPEG")

This tag is not correct.  I think it's actually:
Fixes: a0ccc717c4ab ("drm/amdgpu/discovery: validate VCN and SDMA instances")

With that fixed, the patch is:
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

> Cc: Christian König <christian.koenig at amd.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
> ---
> v3:
>  - Added fixes tag.
>
>  drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index ef800590c1ab..93c84a1c1d3e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -1282,11 +1282,10 @@ static int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev)
>                                  *     0b10 : encode is disabled
>                                  *     0b01 : decode is disabled
>                                  */
> -                               adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
> -                                       ip->revision & 0xc0;
> -                               ip->revision &= ~0xc0;
>                                 if (adev->vcn.num_vcn_inst <
>                                     AMDGPU_MAX_VCN_INSTANCES) {
> +                                       adev->vcn.vcn_config[adev->vcn.num_vcn_inst] =
> +                                               ip->revision & 0xc0;
>                                         adev->vcn.num_vcn_inst++;
>                                         adev->vcn.inst_mask |=
>                                                 (1U << ip->instance_number);
> @@ -1297,6 +1296,7 @@ static int amdgpu_discovery_reg_base_init(struct amdgpu_device *adev)
>                                                 adev->vcn.num_vcn_inst + 1,
>                                                 AMDGPU_MAX_VCN_INSTANCES);
>                                 }
> +                               ip->revision &= ~0xc0;
>                         }
>                         if (le16_to_cpu(ip->hw_id) == SDMA0_HWID ||
>                             le16_to_cpu(ip->hw_id) == SDMA1_HWID ||
> --
> 2.34.1
>