Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Integer overflow in `roundup_pow_of_two` on 32-bit systems causes a massive heap buffer overflow when allocating physically contiguous pages for large objects. -- commit bdaf5470cc890b63dd50b4ef6b0bd2b13dcdaa7b Author: Joonas Lahtinen <[email protected]> drm/i915/gem: Fix phys BO pread/pwrite with offset This commit fixes a scaling issue in phys BO pread/pwrite when using non-zero offsets. It updates the code to correctly access the struct page pointers via scatterlist abstractions instead of void pointers. > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_phys.c > b/drivers/gpu/drm/i915/gem/i915_gem_phys.c > index e375afbf458ee..d53129eb56030 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_phys.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_phys.c [ ... ] > @@ -58,7 +69,7 @@ static int i915_gem_object_get_pages_phys(struct > drm_i915_gem_object *obj) > sg->offset = 0; > sg->length = obj->base.size; > > - sg_assign_page(sg, (struct page *)vaddr); > + __set_phys_vaddr(sg, vaddr); [Severity: Critical] This is a pre-existing issue, but does the dma_alloc_coherent() allocation earlier in this function overflow on 32-bit systems? Looking at i915_gem_object_get_pages_phys(), we allocate memory using: vaddr = dma_alloc_coherent(obj->base.dev->dev, roundup_pow_of_two(obj->base.size), &dma, GFP_KERNEL); If a user allocates a GEM object larger than 2GB (e.g., 0x80001000), roundup_pow_of_two() calculates 1UL << fls_long(n - 1). On x86 32-bit, shift amounts are masked to 5 bits, so 32 & 31 = 0, resulting in an allocation size of 1 byte (which the allocator rounds up to PAGE_SIZE). Later in the same function, a loop writes obj->base.size bytes into this undersized buffer: for (i = 0; i < obj->base.size / PAGE_SIZE; i++) { struct page *page; page = shmem_read_mapping_page(mapping, i); if (IS_ERR(page)) goto err_st; memcpy_from_page(dst, page, 0, PAGE_SIZE); drm_clflush_virt_range(dst, PAGE_SIZE); put_page(page); dst += PAGE_SIZE; } Can this cause a massive buffer overflow and corrupt physical memory when handling large objects? > sg_dma_address(sg) = dma; > sg_dma_len(sg) = obj->base.size; > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
