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

[Kamil Konieczny]

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 at 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.

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
> >