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 (see shmem_get_pages(),
rebuild_st label for context), were not enough to jump to the
next folio, which has never been a problem before folios have
been introduced. In result, sg_set_folio() was called multiple
times with nr_pages smaller than folio size, using multitude of
scatterlists, all pointing to the beginning pages of the folio
and never fully covering its range of pages.

Track how many pages have already been counted in a folio to
ensure it is fully covered before reading next folio.

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 <[email protected]>
---
 drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 43 ++++++++++++++++-------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c 
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index b5ae7e5f80a0..0e4929efbfe5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -138,7 +138,10 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, 
struct sg_table *st,
 {
        unsigned int pages_left; /* restricted by sg_alloc_table */
        unsigned long next_pfn = 0; /* suppress gcc warning */
+       unsigned long folio_start = 0;
        unsigned long pages_done = 0;
+       unsigned long folio_end = 0;
+       struct folio *folio = NULL;
        struct scatterlist *sg;
        gfp_t noreclaim;
        int ret;
@@ -166,37 +169,53 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915, 
struct sg_table *st,
        sg = st->sgl;
 
        while (pages_left) {
+               unsigned long folio_pages_done = 0;
                unsigned long nr_pages;
                gfp_t gfp = noreclaim;
-               struct folio *folio;
 
-               folio = shmem_shrink_get_folio(mapping, pages_done, gfp,
-                                              pages_left, i915);
-               if (IS_ERR(folio)) {
-                       ret = PTR_ERR(folio);
+               /* Grab the next folio if we exhausted the current one. */
+               if (!pages_done || pages_done > folio_end) {
+                       folio = shmem_shrink_get_folio(mapping, pages_done, gfp,
+                                                      pages_left, i915);
+                       if (IS_ERR(folio)) {
+                               ret = PTR_ERR(folio);
+                               goto err_sg;
+                       }
+
+                       folio_start = folio_pgoff(folio);
+                       folio_end = folio_start + folio_nr_pages(folio) - 1;
+               }
+
+               folio_pages_done = pages_done - folio_start;
+               if (WARN_ON_ONCE(folio_pages_done >= folio_nr_pages(folio))) {
+                       ret = -EINVAL;
+                       folio_put(folio);
                        goto err_sg;
                }
 
                nr_pages = min_array(((unsigned long[]){
-                                            folio_nr_pages(folio),
+                                            folio_nr_pages(folio) - 
folio_pages_done,
                                             pages_left,
-                                            max_segment / PAGE_SIZE,
+                                            max_t(unsigned int, 1, max_segment 
/ PAGE_SIZE),
                                     }), 3);
                if (!st->nents) {
                        st->nents++;
-                       sg_set_folio(sg, folio, nr_pages * PAGE_SIZE, 0);
+                       sg_set_page(sg, folio_page(folio, 0), nr_pages * 
PAGE_SIZE, 0);
                } else if (sg->length >= max_segment ||
-                          folio_pfn(folio) != next_pfn) {
+                          folio_pfn(folio) + folio_pages_done != next_pfn) {
                        sg = sg_next(sg);
                        st->nents++;
-                       sg_set_folio(sg, folio, nr_pages * PAGE_SIZE, 0);
+                       sg_set_page(sg, folio_page(folio, folio_pages_done),
+                                   nr_pages * PAGE_SIZE, 0);
                } else {
                        nr_pages = min_t(unsigned long, nr_pages,
-                                        (max_segment - sg->length) / 
PAGE_SIZE);
+                                               max_t(unsigned long, 1,
+                                                     (max_segment - 
sg->length) / PAGE_SIZE));
 
                        sg->length += nr_pages * PAGE_SIZE;
                }
-               next_pfn = folio_pfn(folio) + nr_pages;
+
+               next_pfn = folio_pfn(folio) + folio_pages_done + nr_pages;
                pages_done += nr_pages;
                pages_left -= nr_pages;
 
-- 
2.34.1

Reply via email to