Re: [PATCH] drm/i915/gem: Fix phys BO pread/pwrite with offset

[sashiko-bot@kernel.org]

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 <joonas.lahtinen@linux.intel.com>

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/20260610060314.26111-1-joonas.lahtinen@linux.intel.com?part=1