Re: [RFC 1/2] drm/i915/shmem: Prevent overflows on small segments

[Sebastian Brzezinka <sebastian.brzezinka@intel.com>]

Hi Krzysztof,

On Wed May 27, 2026 at 4:08 PM CEST, Krzysztof Karas wrote:
> With addition of commit 029ae067431a
> ("drm/i915: Fix potential overflow of shmem scatterlist length")
> max_segment size was included in calculating a number of pages
> for the scatterlist. This meant that segment sizes considerably
> smaller than number of pages in a folio [1], were not enough to
> jump to the next folio. In result, sg_set_folio() was called
> multiple times with nr_pages smaller than folio size, using
> many scatterlists, all pointing to the beginning pages of the
> folio and never fully covering its range of pages and corrupting
> mappings.
>
> [1] See shmem_get_pages(), where segment size is set to
> PAGE_SIZE.
>
> Suggested-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> Fixes: 029ae067431a ("drm/i915: Fix potential overflow of shmem scatterlist length")
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/15816
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> index 06543ae60706..ac9b263c341a 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -156,7 +156,7 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, struct sg_table *st,
>  		nr_pages = min_array(((unsigned long[]) {
>  					folio_nr_pages(folio),
>  					page_count - i,
> -					max_segment / PAGE_SIZE,
> +					i915_sg_segment_size(i915->drm.dev) / PAGE_SIZE,
I don't think we can use i915_sg_segment_size() here, please correct me
if I'm wrong.

When DMA mapping fails, shmem_get_pages() sets max_segment = PAGE_SIZE
and retries. If we replace max_segment with i915_sg_segment_size() in the
min_array, nothing really changes between the first and second attempt,
we end up computing the same nr_pages as before and building the same
sg table that just failed to map.

It also breaks the sg->length check on line 163. After the first folio,
sg->length equals i915_sg_segment_size(). With max_segment=PAGE_SIZE the
check sg->length >= max_segment is always true, never and the PAGE_SIZE
retry has no effect.

-- 
Best regards,
Sebastian