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 <[email protected]> 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/[email protected]?part=1
