Re: [PATCH v2] drm/radeon: switch from get_user_pages to pin_user_pages

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The return value of `radeon_ttm_tt_pin_userptr()` is completely ignored in `radeon_ttm_backend_bind()`, leading to arbitrary DMA mapping, Use-After-Free, and Double Free vulnerabilities.
--

commit 4531c5ac337e9cc383254beba3b239a8be2b493c
Author: Seongjun Hong <hsj0512@snu.ac.kr>
drm/radeon: switch from get_user_pages to pin_user_pages

This commit replaces the deprecated get_user_pages() API with
pin_user_pages() to ensure DMA pages are properly pinned. It also adds the
FOLL_LONGTERM flag to reflect that these userptr pages remain pinned for
the lifetime of the GEM userptr BO's GART binding.

> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index e7ab8162ac694..c14b215c7e3e1 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c

[ ... ]

> @@ -377,7 +378,7 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_device *bdev, struct ttm_tt *ttm
>  	kfree(ttm->sg);
>  
>  release_pages:
> -	release_pages(ttm->pages, pinned);
> +	unpin_user_pages(ttm->pages, pinned);

[Severity: Critical]
This is a pre-existing issue, but does this error path leave ttm->sg as a
dangling pointer?

When pin_user_pages() fails (which could happen more easily now if userspace
provides File System DAX mappings that are cleanly rejected due to the newly
added FOLL_LONGTERM flag), the failure is handled here and ttm->sg is freed
without being set to NULL.

Because radeon_ttm_backend_bind() completely ignores the return value of
radeon_ttm_tt_pin_userptr():

drivers/gpu/drm/radeon/radeon_ttm.c:radeon_ttm_backend_bind() {
    ...
	if (gtt->userptr) {
		radeon_ttm_tt_pin_userptr(bdev, ttm);
		flags &= ~RADEON_GART_PAGE_WRITE;
	}
    ...
}

Execution proceeds with an invalid state, leading to arbitrary GPU DMA access
to physical memory or address 0.

Furthermore, this dangling pointer will be accessed later in
radeon_ttm_tt_unpin_userptr():

drivers/gpu/drm/radeon/radeon_ttm.c:radeon_ttm_tt_unpin_userptr() {
    ...
	/* double check that we don't free the table twice */
	if (!ttm->sg || !ttm->sg->sgl)
		return;
    ...
}

This causes a Use-After-Free read, and subsequently a Double Free when the
BO is destroyed and radeon_ttm_tt_unpopulate() calls kfree(ttm->sg).

Could we set ttm->sg = NULL after freeing it and check the return value of
radeon_ttm_tt_pin_userptr() in radeon_ttm_backend_bind() to prevent this?

>  	return r;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906152726.1685848-1-hsj0512@snu.ac.kr?part=1