Re: [PATCH v2 2/2] drm/xe: Fix stale pinned_link entry when fb-pin performs the final unpin

[Matthew Auld <matthew.auld@intel.com>]

On 01/10/2026 14:40, Thomas Hellström wrote:
> xe_bo_pin_external() and xe_bo_unpin_external() maintain the bo's
> pinned_link list membership in xe->pinned.late.external, based on
> whether the current call is the outermost pin or the final unpin.
> However, the same external bo's pin_count can also be raised and
> lowered directly by __xe_pin_fb_vma()/__xe_unpin_fb_vma(), which pin
> the bo as a display scanout buffer without going through
> xe_bo_pin_external()/xe_bo_unpin_external() at all, and have no
> notion of, or ownership over, pinned_link.
> 
> If a bo is pinned both externally (e.g. dma-buf export) and as an fb,
> and the external unpin happens first, xe_bo_unpin_external() correctly
> observes pin_count > 1 and leaves the bo on pinned_link. When the fb
> unpin later performs the true last unpin (pin_count 1 -> 0), it never
> touches pinned_link, leaving the bo linked on xe->pinned.late.external
> indefinitely. Once the bo is subsequently freed, this stale list entry
> points into freed memory, corrupting the list and risking a
> use-after-free the next time the list is walked or spliced.
> 
> The backup object pin/unpin sites in xe_bo_notifier_prepare_pinned()/
> xe_bo_notifier_unprepare_pinned() have the same bypass characteristic,
> though they never add their bo to a pinned list, so are not affected
> by this particular list-corruption issue.
> 
> Move the pinned_link removal into xe_bo_account_unpin(), which already
> runs on every unpin path (kernel, external, framebuffer, backup
> object) right before the true 1 -> 0 pin_count transition. Since
> list_del_init() only operates on the node itself, this removal is
> list-agnostic and safe to perform regardless of which list (external
> or kernel_bo_present) the bo happens to be linked on, or which code
> path is performing the final unpin. Drop the now-redundant explicit
> list_del_init() calls in xe_bo_unpin_external() and xe_bo_unpin().

I think you can also use this idea to bypass the p2p checking? Create a 
VRAM + TT buffer, turn it into an fb and use it for scanout. While fb 
pinned you dma-buf export/import + map it (dynamic). AFAICT you can 
trick xe_dma_buf_map() since this buffer looks like you can migrate it 
(dual placement) but since already fb pinned this will make it skip the 
actual migrate/validate, so it falls through to mapping the VRAM using 
the pci address. However, this could be on a system that lacks p2p 
support. Don't see what prevents that?

> 
> Fixes: 44e694958b95 ("drm/xe/display: Implement display support")
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: intel-xe@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v6.8+
> Assisted-by: LLM
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> 
> v2:
> - New patch
> ---
>   drivers/gpu/drm/xe/xe_bo.c | 25 ++++++++++++++++---------
>   1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index 2fbbba7cf4b0..581bfded21db 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -487,12 +487,27 @@ static void xe_bo_account_pin(struct xe_bo *bo)
>    * held, before &ttm_buffer_object.pin_count is decremented by
>    * ttm_bo_unpin(), so that the check against the true 1->0 transition sees
>    * the pin count that is about to be released.
> + *
> + * On the true last unpin, also removes @bo from whichever pinned-bo list
> + * (external or kernel_bo_present) it may currently be linked on, since a
> + * bo's final unpin can happen through a pin path (e.g. framebuffer,
> + * backup object) that has no notion of, or ownership over, that list.
> + * This is safe and list-agnostic: list_del_init() only needs the node
> + * itself, not knowledge of which list it is threaded through, and is a
> + * no-op if @bo is not linked.
>    */
>   static void xe_bo_account_unpin(struct xe_bo *bo)
>   {
>   	struct xe_device *xe = xe_bo_device(bo);
> +	bool last_unpin = bo->ttm.pin_count == 1;
>   
> -	if (bo->ttm.pin_count == 1 && bo->ttm.ttm && ttm_tt_is_populated(bo->ttm.ttm))
> +	if (last_unpin && !list_empty(&bo->pinned_link)) {
> +		spin_lock(&xe->pinned.lock);
> +		list_del_init(&bo->pinned_link);
> +		spin_unlock(&xe->pinned.lock);
> +	}
> +
> +	if (last_unpin && bo->ttm.ttm && ttm_tt_is_populated(bo->ttm.ttm))
>   		xe_ttm_tt_account_add(xe, bo->ttm.ttm);
>   }
>   
> @@ -3311,11 +3326,6 @@ void xe_bo_unpin_external(struct xe_bo *bo)
>   	xe_assert(xe, xe_bo_is_pinned(bo));
>   	xe_assert(xe, xe_bo_is_user(bo));
>   
> -	spin_lock(&xe->pinned.lock);
> -	if (bo->ttm.pin_count == 1 && !list_empty(&bo->pinned_link))
> -		list_del_init(&bo->pinned_link);
> -	spin_unlock(&xe->pinned.lock);
> -
>   	xe_bo_unpin_account(bo);
>   
>   	/*
> @@ -3337,10 +3347,7 @@ void xe_bo_unpin(struct xe_bo *bo)
>   	xe_assert(xe, xe_bo_is_pinned(bo));
>   
>   	if (mem_type_is_vram(place->mem_type) || bo->flags & XE_BO_FLAG_GGTT) {
> -		spin_lock(&xe->pinned.lock);
>   		xe_assert(xe, !list_empty(&bo->pinned_link));
> -		list_del_init(&bo->pinned_link);
> -		spin_unlock(&xe->pinned.lock);
>   
>   		if (bo->backup_obj) {
>   			if (xe_bo_is_pinned(bo->backup_obj))