Re: [PATCH] drm/i915: Fix potential overflow of shmem scatterlist length

[Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>]

Hi Andi,

Thanks for review.

On Wednesday, 25 February 2026 15:41:13 CET Andi Shyti wrote:
> Hi Janusz,
> 
> ...
> 
> > @@ -153,8 +153,12 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
> >  			}
> >  		} while (1);
> >  
> 
> Perhaps we could add here:
> 
> 	max_pages = max_segment >> PAGE_SHIFT;
> 	/* Just to be paranoic, but not necessary */
> 	if (!max_pages)

GEM_BUG_ON(!max_pages), I would rather say.  The max_segment comes from 
drivers/gpu/drm/i915/i915_scatterlist.h:i915_sg_segment_size(struct device *dev),
let's assume that can't be that much broken.

> 		max_pages = 1;
> 
> 
> > -		nr_pages = min_t(unsigned long,
> > -				folio_nr_pages(folio), page_count - i);
> > +		nr_pages = min_array(((unsigned long[]) {
> > +					folio_nr_pages(folio),
> > +					page_count - i,
> > +					max_segment / PAGE_SIZE,
> 
> max_segment >> PAGE_SHIFT ?

Yeah, shift seems more optimal than division here, however, I've just followed 
the patter used a few lines below. where we can see a multiplication, not a 
right shift, and since PAGE_SIZE is a constant, I hope the compiler will 
optimize that.

> 
> For clarity this can be written as
> 
> 		nr_pages = min_t(unsigned long,
> 				folio_nr_pages(folio), page_count - i);
> 		nr_pages = min_t(unsigned long, nr_pages, max_pages);

Do you think the min_array() is less clear?  Let's see what others say.

> 
> But these are nitpicks, it's then up to you to choose the style.
> 
> Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com>

Thank you,
Janusz

> 
> Thanks,
> Andi
> 
> > +				      }), 3);
> > +
>