Re: [PATCH 6.1.y] drm/amdgpu: Fix potential out-of-bounds access in 'amdgpu_discovery_reg_base_init()'

[Christian König]

Hi Li,

On 3/23/26 08:10, Li hongliang wrote:
> From: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>
> 
> [ Upstream commit cdb637d339572398821204a1142d8d615668f1e9 ]
> 
> 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.

well this patch only fixed a compiler warning and has not much practical value otherwise.

Why are you sending this for inclusion into the 6.1 kernel?

Regards,
Christian.

> 
> Fixes: a0ccc717c4ab ("drm/amdgpu/discovery: validate VCN and SDMA instances")
> Cc: Christian König <christian.koenig@xxxxxxx>
> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>
> Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> [ Minor conflict resolved. ]
> Signed-off-by: Li hongliang <1468888505@xxxxxxx>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> index 9b1c4d5be61f..a1e006d238cf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> @@ -1128,15 +1128,15 @@ 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)
> +				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++;
> -				else
> +				} else
>  					dev_err(adev->dev, "Too many VCN instances: %d vs %d\n",
>  						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 ||