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

On 2026-07-23 06:55, Dan Carpenter wrote:
> [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> 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.
That catches the issue earlier which can save some CPU cycles but amdgpu_vce_validate_bo() is supposed to be resilient to bad addresses since the user could just put bogus addresses into the IB. In this case we just need to prevent a partial address patch, i.e. the hi part of the address got patched with the physical address but the lo part did not (because it's outside the IB).
>
> regards,
> dan carpenter
>