When a scatterlists table of a GEM shmem object of size 4 GB or more is
populated with pages allocated from a folio, unsigned int .length
attribute of a scatterlist may get overflowed if total byte length of
pages allocated to that single scatterlist happens to reach or cross the
4GB limit. As a consequence, users of the object may suffer from hitting
unexpected, premature end of the object's backing pages.
[278.780187] ------------[ cut here ]------------
[278.780377] WARNING: CPU: 1 PID: 2326 at drivers/gpu/drm/i915/i915_mm.c:55
remap_sg+0x199/0x1d0 [i915]
...
[278.780654] CPU: 1 UID: 0 PID: 2326 Comm: gem_mmap_offset Tainted: G S U
6.17.0-rc1-CI_DRM_16981-ged823aaa0607+ #1 PREEMPT(voluntary)
[278.780656] Tainted: [S]=CPU_OUT_OF_SPEC, [U]=USER
[278.780658] Hardware name: Intel Corporation Meteor Lake Client Platform/MTL-P
LP5x T3 RVP, BIOS MTLPFWI1.R00.3471.D91.2401310918 01/31/2024
[278.780659] RIP: 0010:remap_sg+0x199/0x1d0 [i915]
...
[278.780786] Call Trace:
[278.780787] <TASK>
[278.780788] ? __apply_to_page_range+0x3e6/0x910
[278.780795] ? __pfx_remap_sg+0x10/0x10 [i915]
[278.780906] apply_to_page_range+0x14/0x30
[278.780908] remap_io_sg+0x14d/0x260 [i915]
[278.781013] vm_fault_cpu+0xd2/0x330 [i915]
[278.781137] __do_fault+0x3a/0x1b0
[278.781140] do_fault+0x322/0x640
[278.781143] __handle_mm_fault+0x938/0xfd0
[278.781150] handle_mm_fault+0x12c/0x300
[278.781152] ? lock_mm_and_find_vma+0x4b/0x760
[278.781155] do_user_addr_fault+0x2d6/0x8e0
[278.781160] exc_page_fault+0x96/0x2c0
[278.781165] asm_exc_page_fault+0x27/0x30
...
That issue was apprehended by the author of a change that introduced it,
and potential risk even annotated with a comment, but then never addressed.
When adding folio pages to a scatterlist table, take care of byte length
of any single scatterlist not exceeding max_segment.
Fixes: 0b62af28f249b ("i915: convert shmem_sg_free_table() to use a
folio_batch")
Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14809
Cc: Matthew Wilcox (Oracle) <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: [email protected] # v6.5+
Signed-off-by: Janusz Krzysztofik <[email protected]>
---
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index c6c64ba29bc42..720a9ad39aa2a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -153,8 +153,12 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915,
struct sg_table *st,
}
} while (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,
+ }), 3);
+
if (!i ||
sg->length >= max_segment ||
folio_pfn(folio) != next_pfn) {
@@ -164,7 +168,9 @@ int shmem_sg_alloc_table(struct drm_i915_private *i915,
struct sg_table *st,
st->nents++;
sg_set_folio(sg, folio, nr_pages * PAGE_SIZE, 0);
} else {
- /* XXX: could overflow? */
+ nr_pages = min_t(unsigned long, nr_pages,
+ (max_segment - sg->length) /
PAGE_SIZE);
+
sg->length += nr_pages * PAGE_SIZE;
}
next_pfn = folio_pfn(folio) + nr_pages;
--
2.52.0
