Re: [igt-dev] [PATCH i-g-t] lib/intel_blt.c: ensure uint64_t result of multiplication

["Bernatowicz, Marcin" <marcin.bernatowicz@linux.intel.com>]

On 10/19/2023 5:27 PM, Kamil Konieczny wrote:
> Hi Marcin,
> 
> On 2023-10-18 at 17:28:11 +0200, Kamil Konieczny wrote:
>> Hi Marcin,
>> On 2023-10-17 at 14:36:54 +0000, Marcin Bernatowicz wrote:
>>> Additionally check for overflow.
>> - ^^^^^^^^^^^^
>> This type was from the start uint64, so imho change subject from:
>>
>> lib/intel_blt.c: ensure uint64_t result of multiplication
>> ------------ ^^
>> sidenote: remove ".c"
>>
>> into:
>> lib/intel_blt: check for overflow in multiplication
>>
>> and adjust description.
>>
>>>
>>> This should allow to exercise large buffers
>>> ex. xe_exercise_blt -W 16384 -H 16384
>>
>> Please explain - this should fit in 32bit? 16K*16K*32 = 0x40000000
>> Or do you mean much higher values for W and H?
>>
> 
> You were right here, sorry.
> 
>>>
>>> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
>>> ---
>>>   lib/intel_blt.c | 8 +++++++-
>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/intel_blt.c b/lib/intel_blt.c
>>> index a76c7a404..f46c85e91 100644
>>> --- a/lib/intel_blt.c
>>> +++ b/lib/intel_blt.c
>>> @@ -1607,12 +1607,18 @@ blt_create_object(const struct blt_copy_data *blt, uint32_t region,
>>>   		  bool create_mapping)
>>>   {
>>>   	struct blt_copy_object *obj;
>>> -	uint64_t size = width * height * bpp / 8;
>>>   	uint32_t stride = tiling == T_LINEAR ? width * 4 : width;
>>>   	uint32_t handle;
>>> +	uint64_t size;
>>>   
>>>   	igt_assert_f(blt->driver, "Driver isn't set, have you called blt_copy_init()?\n");
>>>   
>>> +	igt_assert_f((UINT64_MAX / 8) >= width &&
>> ----------------- ^^^^^^^^^^^^^^
>> This is not needed, it checks for MAX >= w * 8, while you want
>> size > 0, imho add a second assert after calculating size.
>>
>> Regards,
>> Kamil
>>
> 
> One more thing, before these asserts you should check that
> both width and height are not zero.

True, more asserts needed :)

> 
> Regards,
> Kamil
> 
>>> +		     (UINT64_MAX / width) >= height &&
>>> +		     (UINT64_MAX / (width * height)) >= bpp, "Overflow detected!\n");
>>> +
>>> +	size = (uint64_t)width * height * bpp / 8;
>>> +
>>>   	obj = calloc(1, sizeof(*obj));
>>>   
>>>   	obj->size = size;
>>> -- 
>>> 2.42.0
>>>