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
>
