Re: [RFC PATCH v1 2/2] drm/msm: reject dma-buf imports without struct page info

[Bryan O'Donoghue]

On 23/09/2026 08:42, Jianfeng Liu wrote:
> msm_gem_import() fills the GEM object's page array with the deprecated
> drm_prime_sg_to_page_array() and stores the attachment sg_table for
> later mapping into the GPU's own pagetables via iommu_map_sgtable().
> Both need the struct page of the sg_table:
> 
>   - iommu_map_sg() maps sg_phys() of each entry, and
>   - drm_prime_sg_to_page_array() iterates with for_each_sgtable_page,
>     which walks sg->length.
> 
> When CONFIG_DMABUF_DEBUG=y, dma_buf_map_attachment() hands importers
> a copy of the sg_table with the page pointers stripped and sg->length
> zeroed.  In that case drm_prime_sg_to_page_array() "succeeds" while
> filling zero entries, leaving msm_obj->pages uninitialized garbage
> (kvmalloc_objs() does not zero).  The buffer is imported anyway, and
> the first VM_BIND map of it fails asynchronously in the scheduler job
> run - after userspace has already enqueued GPU work referencing the
> mapping.  Userspace then observes arm-smmu translation faults from
> UCHE, e.g. hardware video decode in clapper/chromium:
> 
>    gpu fault: ttbr0=000000088a889000 iova=000000010741c000 dir=READ
>    type=TRANSLATION source=UCHE
> 
> Replace the deprecated helper with an explicit loop so that a missing
> or short page list is detected at import time and rejected with
> -EINVAL.  This turns the silent memory corruption into a clean import
> error, letting userspace fall back instead of crashing the GPU.
> 
> Note that msm fundamentally cannot map a page-less sg_table into its
> per-process GPU pagetables (it needs the physical addresses), so
> imports of such buffers can never work until msm is converted to
> build its GPU mappings from the attachment's DMA addresses.
> 
> Signed-off-by: Jianfeng Liu <liujianfeng1994@xxxxxxxxx>
> 
> ---
> 
>   drivers/gpu/drm/msm/msm_gem.c | 31 ++++++++++++++++++++++++++++---
>   1 file changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index c4cff3d53d81b..0d5a91181d05b 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -1307,7 +1307,8 @@ struct drm_gem_object *msm_gem_import(struct drm_device *dev,
>   	struct msm_gem_object *msm_obj;
>   	struct drm_gem_object *obj;
>   	struct dma_buf *dmabuf = attach->dmabuf;
> -	size_t size, npages;
> +	struct sg_page_iter piter;
> +	size_t size, npages, filled = 0;
>   	int ret;
> 
>   	size = PAGE_ALIGN(dmabuf->size);
> @@ -1333,8 +1334,32 @@ struct drm_gem_object *msm_gem_import(struct drm_device *dev,
>   		goto fail;
>   	}
> 
> -	ret = drm_prime_sg_to_page_array(sgt, msm_obj->pages, npages);
> -	if (ret) {
> +	/*
> +	 * Fill the page array ourselves instead of using the deprecated
> +	 * drm_prime_sg_to_page_array(), so that we can detect sg_tables
> +	 * that carry no struct page at all.  Those must be rejected:
> +	 * msm maps imported buffers into the GPU's own pagetables with
> +	 * iommu_map_sgtable(), which needs the physical pages, so an
> +	 * import without page information could never be mapped.  The
> +	 * most prominent case is the page-stripping sg_table wrapper that
> +	 * dma_buf_map_attachment() hands out when CONFIG_DMABUF_DEBUG=y.
> +	 *
> +	 * drm_prime_sg_to_page_array() would "succeed" with zero entries
> +	 * filled in that case and leave msm_obj->pages uninitialized,
> +	 * which later blows up as arm-smmu translation faults from UCHE.
> +	 */
> +	for_each_sgtable_page(sgt, &piter, 0) {
> +		if (WARN_ON(filled >= npages)) {
> +			ret = -EINVAL;
> +			goto fail;
> +		}
> +		msm_obj->pages[filled++] = sg_page_iter_page(&piter);
> +	}
> +	if (filled != npages) {
> +		DRM_DEV_ERROR(dev->dev,
> +			      "import of dmabuf from '%s' rejected: sg_table has no/misaligned struct page info\n",
> +			      dmabuf->exp_name ?: "?");
> +		ret = -EINVAL;
>   		goto fail;
>   	}
> 
> --
> 2.47.3
> 
> 

This very much looks like an LLM generated patch - the commit log, the 
large comment in the code and TBH the solution too.

Why is the fix Adreno specific ?

Shouldn't this function be ammended with

 > +	if (filled != npages)

instead ?

/**
  * drm_prime_sg_to_page_array - convert an sg table into a page array
  * @sgt: scatter-gather table to convert
  * @pages: array of page pointers to store the pages in
  * @max_entries: size of the passed-in array
  *
  * Exports an sg table into an array of pages.
  *
  * This function is deprecated and strongly discouraged to be used.
  * The page array is only useful for page faults and those can corrupt 
fields
  * in the struct page if they are not handled by the exporting driver.
  */
int __deprecated drm_prime_sg_to_page_array(struct sg_table *sgt,
                                             struct page **pages,
                                             int max_entries)
{
         struct sg_page_iter page_iter;
         struct page **p = pages;

         for_each_sgtable_page(sgt, &page_iter, 0) {
                 if (WARN_ON(p - pages >= max_entries))
                         return -1;
                 *p++ = sg_page_iter_page(&page_iter);
         }
         return 0;
}
EXPORT_SYMBOL(drm_prime_sg_to_page_array);

All the LLM seems to have done here is copy the code out of 
drm_prime_sg_to_page_array() and then add a check for filled != npages

But if that is a valid check for Adreno - then it is a valid check for:

  grep drm_prime_sg_to_page_array drivers/* -r
drivers/gpu/drm/vmwgfx/vmwgfx_blit.c:		ret = 
drm_prime_sg_to_page_array(src->ttm->sg, src_pages,
drivers/gpu/drm/vmwgfx/vmwgfx_blit.c:		ret = 
drm_prime_sg_to_page_array(dst->ttm->sg, dst_pages,
drivers/gpu/drm/omapdrm/omap_gem.c:		ret = 
drm_prime_sg_to_page_array(sgt, pages, npages);
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c:	ret = 
drm_prime_sg_to_page_array(sgt, etnaviv_obj->pages, npages);
drivers/gpu/drm/drm_prime.c: * drm_prime_sg_to_page_array - convert an 
sg table into a page array
drivers/gpu/drm/drm_prime.c:int __deprecated 
drm_prime_sg_to_page_array(struct sg_table *sgt,
drivers/gpu/drm/drm_prime.c:EXPORT_SYMBOL(drm_prime_sg_to_page_array);
drivers/gpu/drm/msm/msm_gem.c:	ret = drm_prime_sg_to_page_array(sgt, 
msm_obj->pages, npages);
drivers/gpu/drm/xen/xen_drm_front_gem.c:	ret = 
drm_prime_sg_to_page_array(sgt, xen_obj->pages,

and should live in the helper function with an appropriate Fixes: tag 
for backporting...

---
bod