Re: [PATCH v4 2/5] drm/amdgpu/vce: Prevent partial address patches

On Mon, Mar 30, 2026 at 03:57:54PM -0400, Benjamin Cheng wrote:
> In the case that only one of lo/hi is valid, the patching could result
> in a bad address written to in FW.
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index eb4a15db2ef2..efdebd9c0a1f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -680,6 +680,9 @@ static int amdgpu_vce_cs_reloc(struct amdgpu_cs_parser *p, struct amdgpu_ib *ib,
>  	uint64_t addr;
>  	int r;
>  
> +	if (lo >= ib->length_dw || hi >= ib->length_dw)
> +		return -EINVAL;
> +

I feel like the better fix would be to change the amdgpu_ib_get_value()
function to return negative for invalid indexes instead of returning 0.

drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
   562  static inline u32 amdgpu_ib_get_value(struct amdgpu_ib *ib, uint32_t idx)
   563  {
   564          if (idx < ib->length_dw)
   565                  return ib->ptr[idx];
   566          return 0;

s/return 0/return -EINVAL/

   567  }

amdgpu_vce_validate_bo() has the same issue where invalid lo hi are
treated as success.

regards,
dan carpenter