linux-kernel - Re: [PATCH] drm/msm: Fix GEM free for imported dma-bufs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5ab4d0a3-6c58-49b1-93b8-7a69b150c1a9@linaro.org>
Date: Mon, 6 Oct 2025 12:12:23 +0100
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Rob Clark <robin.clark@....qualcomm.com>, dri-devel@...ts.freedesktop.org
Cc: freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
 Stephan Gerhold <stephan.gerhold@...aro.org>,
 Dmitry Baryshkov <lumag@...nel.org>, Abhinav Kumar
 <abhinav.kumar@...ux.dev>, Jessica Zhang <jessica.zhang@....qualcomm.com>,
 Sean Paul <sean@...rly.run>, Marijn Suijten <marijn.suijten@...ainline.org>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/msm: Fix GEM free for imported dma-bufs

On 23/09/2025 15:04, Rob Clark wrote:
> Imported dma-bufs also have obj->resv != &obj->_resv.  So we should
> check both this condition in addition to flags for handling the
> _NO_SHARE case.
> 
> Fixes this splat that was reported with IRIS video playback:
> 
>      ------------[ cut here ]------------
>      WARNING: CPU: 3 PID: 2040 at drivers/gpu/drm/msm/msm_gem.c:1127 msm_gem_free_object+0x1f8/0x264 [msm]
>      CPU: 3 UID: 1000 PID: 2040 Comm: .gnome-shell-wr Not tainted 6.17.0-rc7 #1 PREEMPT
>      pstate: 81400005 (Nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
>      pc : msm_gem_free_object+0x1f8/0x264 [msm]
>      lr : msm_gem_free_object+0x138/0x264 [msm]
>      sp : ffff800092a1bb30
>      x29: ffff800092a1bb80 x28: ffff800092a1bce8 x27: ffffbc702dbdbe08
>      x26: 0000000000000008 x25: 0000000000000009 x24: 00000000000000a6
>      x23: ffff00083c72f850 x22: ffff00083c72f868 x21: ffff00087e69f200
>      x20: ffff00087e69f330 x19: ffff00084d157ae0 x18: 0000000000000000
>      x17: 0000000000000000 x16: ffffbc704bd46b80 x15: 0000ffffd0959540
>      x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
>      x11: ffffbc702e6cdb48 x10: 0000000000000000 x9 : 000000000000003f
>      x8 : ffff800092a1ba90 x7 : 0000000000000000 x6 : 0000000000000020
>      x5 : ffffbc704bd46c40 x4 : fffffdffe102cf60 x3 : 0000000000400032
>      x2 : 0000000000020000 x1 : ffff00087e6978e8 x0 : ffff00087e6977e8
>      Call trace:
>       msm_gem_free_object+0x1f8/0x264 [msm] (P)
>       drm_gem_object_free+0x1c/0x30 [drm]
>       drm_gem_object_handle_put_unlocked+0x138/0x150 [drm]
>       drm_gem_object_release_handle+0x5c/0xcc [drm]
>       drm_gem_handle_delete+0x68/0xbc [drm]
>       drm_gem_close_ioctl+0x34/0x40 [drm]
>       drm_ioctl_kernel+0xc0/0x130 [drm]
>       drm_ioctl+0x360/0x4e0 [drm]
>       __arm64_sys_ioctl+0xac/0x104
>       invoke_syscall+0x48/0x104
>       el0_svc_common.constprop.0+0x40/0xe0
>       do_el0_svc+0x1c/0x28
>       el0_svc+0x34/0xec
>       el0t_64_sync_handler+0xa0/0xe4
>       el0t_64_sync+0x198/0x19c
>      ---[ end trace 0000000000000000 ]---
>      ------------[ cut here ]------------
> 
> Reported-by: Stephan Gerhold <stephan.gerhold@...aro.org>
> Fixes: de651b6e040b ("drm/msm: Fix refcnt underflow in error path")
> Signed-off-by: Rob Clark <robin.clark@....qualcomm.com>
> ---
>   drivers/gpu/drm/msm/msm_gem.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index 9f0f5b77f1bd..3aea9b493375 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -1121,12 +1121,16 @@ static void msm_gem_free_object(struct drm_gem_object *obj)
>   		put_pages(obj);
>   	}
> 
> -	if (obj->resv != &obj->_resv) {
> +	/*
> +	 * In error paths, we could end up here before msm_gem_new_handle()
> +	 * has changed obj->resv to point to the shared resv.  In this case,
> +	 * we don't want to drop a ref to the shared r_obj that we haven't
> +	 * taken yet.
> +	 */
> +	if ((msm_obj->flags & MSM_BO_NO_SHARE) && (obj->resv != &obj->_resv)) {
>   		struct drm_gem_object *r_obj =
>   			container_of(obj->resv, struct drm_gem_object, _resv);
> 
> -		WARN_ON(!(msm_obj->flags & MSM_BO_NO_SHARE));
> -
>   		/* Drop reference we hold to shared resv obj: */
>   		drm_gem_object_put(r_obj);
>   	}
> --
> 2.51.0
> 
> 
Tested-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org> # qrb5165-rb5