* [PATCH 0/2] drm/msm: Error path fixes
@ 2025-07-23 19:08 Rob Clark
2025-07-23 19:08 ` [PATCH 1/2] drm/msm: Fix refcnt underflow in error path Rob Clark
2025-07-23 19:08 ` [PATCH 2/2] drm/msm: Fix submit error path cleanup Rob Clark
0 siblings, 2 replies; 8+ messages in thread
From: Rob Clark @ 2025-07-23 19:08 UTC (permalink / raw)
To: dri-devel
Cc: linux-arm-msm, freedreno, Rob Clark, Abhinav Kumar, David Airlie,
Dmitry Baryshkov, Jessica Zhang, open list, Marijn Suijten,
Sean Paul, Simona Vetter
For reasons unknown to me, systemd-udev recently started limiting
max-files to 64k (at least in f42), which exposed some problematic
allocation related error paths.
Rob Clark (2):
drm/msm: Fix refcnt underflow in error path
drm/msm: Fix submit error path cleanup
drivers/gpu/drm/msm/msm_gem.c | 4 +++-
drivers/gpu/drm/msm/msm_gem_submit.c | 9 +++++----
2 files changed, 8 insertions(+), 5 deletions(-)
--
2.50.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] drm/msm: Fix refcnt underflow in error path
2025-07-23 19:08 [PATCH 0/2] drm/msm: Error path fixes Rob Clark
@ 2025-07-23 19:08 ` Rob Clark
2025-09-22 16:32 ` Stephan Gerhold
2025-07-23 19:08 ` [PATCH 2/2] drm/msm: Fix submit error path cleanup Rob Clark
1 sibling, 1 reply; 8+ messages in thread
From: Rob Clark @ 2025-07-23 19:08 UTC (permalink / raw)
To: dri-devel
Cc: linux-arm-msm, freedreno, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, open list
If we hit an error path in GEM obj creation before msm_gem_new_handle()
updates obj->resv to point to the gpuvm resv object, then obj->resv
still points to &obj->_resv. In this case we don't want to decrement
the refcount of the object being freed (since the refcnt is already
zero). This fixes the following splat:
------------[ cut here ]------------
refcount_t: underflow; use-after-free.
WARNING: CPU: 9 PID: 7013 at lib/refcount.c:28 refcount_warn_saturate+0xf4/0x148
Modules linked in: uinput snd_seq_dummy snd_hrtimer aes_ce_ccm snd_soc_wsa884x regmap_sdw q6prm_clocks q6apm_lpass_da>
qcom_pil_info i2c_hid drm_kms_helper qcom_common qcom_q6v5 phy_snps_eusb2 qcom_geni_serial drm qcom_sysmon pinctrl_s>
CPU: 9 UID: 1000 PID: 7013 Comm: deqp-vk Not tainted 6.16.0-rc4-debug+ #25 PREEMPT(voluntary)
Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN53WW 08/02/2024
pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
pc : refcount_warn_saturate+0xf4/0x148
lr : refcount_warn_saturate+0xf4/0x148
sp : ffff8000a2073920
x29: ffff8000a2073920 x28: 0000000000000010 x27: 0000000000000010
x26: 0000000000000042 x25: ffff000810e09800 x24: 0000000000000010
x23: ffff8000a2073b94 x22: ffff000ddb22de00 x21: ffff000ddb22dc00
x20: ffff000ddb22ddf8 x19: ffff0008024934e0 x18: 000000000000000a
x17: 0000000000000000 x16: ffff9f8c67d77340 x15: 0000000000000000
x14: 00000000ffffffff x13: 2e656572662d7265 x12: 7466612d65737520
x11: 3b776f6c66726564 x10: 00000000ffff7fff x9 : ffff9f8c67506c70
x8 : ffff9f8c69fa26f0 x7 : 00000000000bffe8 x6 : c0000000ffff7fff
x5 : ffff000f53e14548 x4 : ffff6082ea2b2000 x3 : ffff0008b86ab080
x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0008b86ab080
Call trace:
refcount_warn_saturate+0xf4/0x148 (P)
msm_gem_free_object+0x248/0x260 [msm]
drm_gem_object_free+0x24/0x40 [drm]
msm_gem_new+0x1c4/0x1e0 [msm]
msm_gem_new_handle+0x3c/0x1a0 [msm]
msm_ioctl_gem_new+0x38/0x70 [msm]
drm_ioctl_kernel+0xc8/0x138 [drm]
drm_ioctl+0x2c8/0x618 [drm]
__arm64_sys_ioctl+0xac/0x108
invoke_syscall.constprop.0+0x64/0xe8
el0_svc_common.constprop.0+0x40/0xe8
do_el0_svc+0x24/0x38
el0_svc+0x54/0x1d8
el0t_64_sync_handler+0x10c/0x138
el0t_64_sync+0x19c/0x1a0
irq event stamp: 3698694
hardirqs last enabled at (3698693): [<ffff9f8c675021dc>] __up_console_sem+0x74/0x90
hardirqs last disabled at (3698694): [<ffff9f8c68ce8164>] el1_dbg+0x24/0x90
softirqs last enabled at (3697578): [<ffff9f8c6744ec5c>] handle_softirqs+0x454/0x4b0
softirqs last disabled at (3697567): [<ffff9f8c67360244>] __do_softirq+0x1c/0x28
---[ end trace 0000000000000000 ]---
Fixes: b58e12a66e47 ("drm/msm: Add _NO_SHARE flag")
Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
drivers/gpu/drm/msm/msm_gem.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 33d3354c6102..958bac4e2768 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -1114,10 +1114,12 @@ static void msm_gem_free_object(struct drm_gem_object *obj)
put_pages(obj);
}
- if (msm_obj->flags & MSM_BO_NO_SHARE) {
+ if (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.50.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] drm/msm: Fix submit error path cleanup
2025-07-23 19:08 [PATCH 0/2] drm/msm: Error path fixes Rob Clark
2025-07-23 19:08 ` [PATCH 1/2] drm/msm: Fix refcnt underflow in error path Rob Clark
@ 2025-07-23 19:08 ` Rob Clark
1 sibling, 0 replies; 8+ messages in thread
From: Rob Clark @ 2025-07-23 19:08 UTC (permalink / raw)
To: dri-devel
Cc: linux-arm-msm, freedreno, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, open list
submit_unpin_objects() should come before we unlock the objects. This
fixes the splat:
WARNING: CPU: 2 PID: 2171 at drivers/gpu/drm/msm/msm_gem.h:395 msm_gem_unpin_locked+0x8c/0xd8 [msm]
Modules linked in: uinput snd_seq_dummy snd_hrtimer aes_ce_ccm snd_soc_wsa884x regmap_sdw q6prm_clocks q6apm_lpass_dais q6apm_dai snd_q6dsp_common q6prm snd_q6apm qcom_pd_mapper cdc_mbim cdc_wdm cdc_ncm r8153_ecm cdc_ether usbnet sunrpc nls_ascii nls_cp437 vfat fat snd_soc_x1e80100 snd_soc_lpass_rx_macro snd_soc_lpass_tx_macro snd_soc_lpass_va_macro snd_soc_lpass_wsa_macro snd_soc_qcom_common soundwire_qcom snd_soc_lpass_macro_common snd_soc_hdmi_codec snd_soc_qcom_sdw ext4 snd_soc_core snd_compress soundwire_bus snd_pcm_dmaengine snd_seq mbcache jbd2 snd_seq_device snd_pcm pm8941_pwrkey snd_timer r8152 qcom_spmi_temp_alarm industrialio snd lenovo_yoga_slim7x ath12k mii arm_smccc_trng soundcore rng_core evdev loop panel_samsung_atna33xc20 msm ubwc_config drm_client_lib drm_gpuvm drm_exec gpu_sched drm_display_helper pmic_glink_altmode aux_hpd_bridge ucsi_glink qcom_battmgr phy_qcom_qmp_combo ps883x cec aux_bridge drm_dp_aux_bus i2c_hid_of aes_ce_blk drm_kms_helper aes_ce_cipher i2c_hid qcom_q6v5_pas
ghash_ce qcom_pil_info drm sha1_ce qcom_common phy_snps_eusb2 qcom_geni_serial qcom_q6v5 qcom_sysmon pinctrl_sm8550_lpass_lpi lpasscc_sc8280xp sbsa_gwdt mdt_loader gpio_keys pmic_glink i2c_dev efivarfs autofs4
CPU: 2 UID: 1000 PID: 2171 Comm: gnome-shell Not tainted 6.16.0-rc4-debug+ #25 PREEMPT(voluntary)
Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN53WW 08/02/2024
pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
pc : msm_gem_unpin_locked+0x8c/0xd8 [msm]
lr : msm_gem_unpin_locked+0x88/0xd8 [msm]
sp : ffff80009c963820
x29: ffff80009c963820 x28: ffff80009c9639f8 x27: ffff00080552a830
x26: 0000000000000000 x25: ffff0009d5655800 x24: 0000000000000000
x23: 0000000000000000 x22: 0000000000000000 x21: 0000000000000000
x20: ffff000831db5480 x19: ffff000816e74400 x18: 0000000000000000
x17: 0000000000000000 x16: ffffc1396afdd720 x15: 0000000000000000
x14: 0000000000000000 x13: 0000000000000000 x12: ffff0008c065bc00
x11: ffff0008c065c000 x10: 0000000000000000 x9 : ffffc13945b19074
x8 : 0000000000000000 x7 : 0000000000000209 x6 : 0000000000000002
x5 : 0000000000019d01 x4 : ffff0008ba8db080 x3 : 000000000004093f
x2 : ffff3ed5e727f000 x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
msm_gem_unpin_locked+0x8c/0xd8 [msm] (P)
msm_ioctl_gem_submit+0x32c/0x1760 [msm]
drm_ioctl_kernel+0xc8/0x138 [drm]
drm_ioctl+0x2c8/0x618 [drm]
__arm64_sys_ioctl+0xac/0x108
invoke_syscall.constprop.0+0x64/0xe8
el0_svc_common.constprop.0+0x40/0xe8
do_el0_svc+0x24/0x38
el0_svc+0x54/0x1d8
el0t_64_sync_handler+0x10c/0x138
el0t_64_sync+0x19c/0x1a0
irq event stamp: 2185036
hardirqs last enabled at (2185035): [<ffffc1396afeef9c>] _raw_spin_unlock_irqrestore+0x74/0x80
hardirqs last disabled at (2185036): [<ffffc1396afd8164>] el1_dbg+0x24/0x90
softirqs last enabled at (2184778): [<ffffc13969675e44>] fpsimd_restore_current_state+0x3c/0x328
softirqs last disabled at (2184776): [<ffffc13969675e14>] fpsimd_restore_current_state+0xc/0x328
---[ end trace 0000000000000000 ]---
Fixes: 111fdd2198e6 ("drm/msm: drm_gpuvm conversion")
Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
---
drivers/gpu/drm/msm/msm_gem_submit.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 5f8e939a5906..0ac4c199ec93 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -514,14 +514,15 @@ static int submit_reloc(struct msm_gem_submit *submit, struct drm_gem_object *ob
*/
static void submit_cleanup(struct msm_gem_submit *submit, bool error)
{
+ if (error)
+ submit_unpin_objects(submit);
+
if (submit->exec.objects)
drm_exec_fini(&submit->exec);
- if (error) {
- submit_unpin_objects(submit);
- /* job wasn't enqueued to scheduler, so early retirement: */
+ /* if job wasn't enqueued to scheduler, early retirement: */
+ if (error)
msm_submit_retire(submit);
- }
}
void msm_submit_retire(struct msm_gem_submit *submit)
--
2.50.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/msm: Fix refcnt underflow in error path
2025-07-23 19:08 ` [PATCH 1/2] drm/msm: Fix refcnt underflow in error path Rob Clark
@ 2025-09-22 16:32 ` Stephan Gerhold
2025-09-22 16:41 ` Rob Clark
0 siblings, 1 reply; 8+ messages in thread
From: Stephan Gerhold @ 2025-09-22 16:32 UTC (permalink / raw)
To: Rob Clark
Cc: dri-devel, linux-arm-msm, freedreno, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, open list
Hi Rob,
On Wed, Jul 23, 2025 at 12:08:49PM -0700, Rob Clark wrote:
> If we hit an error path in GEM obj creation before msm_gem_new_handle()
> updates obj->resv to point to the gpuvm resv object, then obj->resv
> still points to &obj->_resv. In this case we don't want to decrement
> the refcount of the object being freed (since the refcnt is already
> zero). This fixes the following splat:
>
> ------------[ cut here ]------------
> refcount_t: underflow; use-after-free.
> WARNING: CPU: 9 PID: 7013 at lib/refcount.c:28 refcount_warn_saturate+0xf4/0x148
> Modules linked in: uinput snd_seq_dummy snd_hrtimer aes_ce_ccm snd_soc_wsa884x regmap_sdw q6prm_clocks q6apm_lpass_da>
> qcom_pil_info i2c_hid drm_kms_helper qcom_common qcom_q6v5 phy_snps_eusb2 qcom_geni_serial drm qcom_sysmon pinctrl_s>
> CPU: 9 UID: 1000 PID: 7013 Comm: deqp-vk Not tainted 6.16.0-rc4-debug+ #25 PREEMPT(voluntary)
> Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN53WW 08/02/2024
> pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> pc : refcount_warn_saturate+0xf4/0x148
> lr : refcount_warn_saturate+0xf4/0x148
> sp : ffff8000a2073920
> x29: ffff8000a2073920 x28: 0000000000000010 x27: 0000000000000010
> x26: 0000000000000042 x25: ffff000810e09800 x24: 0000000000000010
> x23: ffff8000a2073b94 x22: ffff000ddb22de00 x21: ffff000ddb22dc00
> x20: ffff000ddb22ddf8 x19: ffff0008024934e0 x18: 000000000000000a
> x17: 0000000000000000 x16: ffff9f8c67d77340 x15: 0000000000000000
> x14: 00000000ffffffff x13: 2e656572662d7265 x12: 7466612d65737520
> x11: 3b776f6c66726564 x10: 00000000ffff7fff x9 : ffff9f8c67506c70
> x8 : ffff9f8c69fa26f0 x7 : 00000000000bffe8 x6 : c0000000ffff7fff
> x5 : ffff000f53e14548 x4 : ffff6082ea2b2000 x3 : ffff0008b86ab080
> x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0008b86ab080
> Call trace:
> refcount_warn_saturate+0xf4/0x148 (P)
> msm_gem_free_object+0x248/0x260 [msm]
> drm_gem_object_free+0x24/0x40 [drm]
> msm_gem_new+0x1c4/0x1e0 [msm]
> msm_gem_new_handle+0x3c/0x1a0 [msm]
> msm_ioctl_gem_new+0x38/0x70 [msm]
> drm_ioctl_kernel+0xc8/0x138 [drm]
> drm_ioctl+0x2c8/0x618 [drm]
> __arm64_sys_ioctl+0xac/0x108
> invoke_syscall.constprop.0+0x64/0xe8
> el0_svc_common.constprop.0+0x40/0xe8
> do_el0_svc+0x24/0x38
> el0_svc+0x54/0x1d8
> el0t_64_sync_handler+0x10c/0x138
> el0t_64_sync+0x19c/0x1a0
> irq event stamp: 3698694
> hardirqs last enabled at (3698693): [<ffff9f8c675021dc>] __up_console_sem+0x74/0x90
> hardirqs last disabled at (3698694): [<ffff9f8c68ce8164>] el1_dbg+0x24/0x90
> softirqs last enabled at (3697578): [<ffff9f8c6744ec5c>] handle_softirqs+0x454/0x4b0
> softirqs last disabled at (3697567): [<ffff9f8c67360244>] __do_softirq+0x1c/0x28
> ---[ end trace 0000000000000000 ]---
>
> Fixes: b58e12a66e47 ("drm/msm: Add _NO_SHARE flag")
> Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/msm_gem.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index 33d3354c6102..958bac4e2768 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -1114,10 +1114,12 @@ static void msm_gem_free_object(struct drm_gem_object *obj)
> put_pages(obj);
> }
>
> - if (msm_obj->flags & MSM_BO_NO_SHARE) {
> + if (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);
> }
This patch seems to break something for direct IRIS/video playback using
dmabuf. I use a simple GStreamer test pipeline for testing IRIS on X1E
(on GNOME, in case that matters):
$ gst-launch-1.0 filesrc location=bbb_sunflower_2160p_60fps_normal.mp4 \
! qtdemux name=d d.video_0 ! h264parse ! v4l2h264dec \
! capture-io-mode=dmabuf ! waylandsink
The video plays fine, but if I try to exit (CTRL+C) the display hangs
for a few seconds and then the console is spammed with pretty much
exactly the messages that you tried to fix here. If I revert this patch,
everything is fine again. It feels like your patch does exactly the
opposite for this use case. :-)
It seems to run into the WARN_ON you added.
Any ideas?
linux-next should have IRIS support for the Slim 7x if you want to try
this for yourself. Or alternatively, there is a backport for 6.17-rc7 in
the Linaro arm64-laptops tree: https://gitlab.com/Linaro/arm64-laptops/linux
You can find the test video here:
https://download.blender.org/demo/movies/BBB/
Thanks,
Stephan
[ 107.430721] ------------[ cut here ]------------
[ 107.435513] WARNING: CPU: 3 PID: 2040 at drivers/gpu/drm/msm/msm_gem.c:1127 msm_gem_free_object+0x1f8/0x264 [msm]
[ 107.630472] CPU: 3 UID: 1000 PID: 2040 Comm: .gnome-shell-wr Not tainted 6.17.0-rc7 #1 PREEMPT
[ 107.630482] pstate: 81400005 (Nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[ 107.630488] pc : msm_gem_free_object+0x1f8/0x264 [msm]
[ 107.676630] lr : msm_gem_free_object+0x138/0x264 [msm]
[ 107.676666] sp : ffff800092a1bb30
[ 107.676668] x29: ffff800092a1bb80 x28: ffff800092a1bce8 x27: ffffbc702dbdbe08
[ 107.676676] x26: 0000000000000008 x25: 0000000000000009 x24: 00000000000000a6
[ 107.676682] x23: ffff00083c72f850 x22: ffff00083c72f868 x21: ffff00087e69f200
[ 107.676689] x20: ffff00087e69f330 x19: ffff00084d157ae0 x18: 0000000000000000
[ 107.676695] x17: 0000000000000000 x16: ffffbc704bd46b80 x15: 0000ffffd0959540
[ 107.676701] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
[ 107.676706] x11: ffffbc702e6cdb48 x10: 0000000000000000 x9 : 000000000000003f
[ 107.676712] x8 : ffff800092a1ba90 x7 : 0000000000000000 x6 : 0000000000000020
[ 107.676718] x5 : ffffbc704bd46c40 x4 : fffffdffe102cf60 x3 : 0000000000400032
[ 107.676724] x2 : 0000000000020000 x1 : ffff00087e6978e8 x0 : ffff00087e6977e8
[ 107.676731] Call trace:
[ 107.676733] msm_gem_free_object+0x1f8/0x264 [msm] (P)
[ 107.676771] drm_gem_object_free+0x1c/0x30 [drm]
[ 107.676816] drm_gem_object_handle_put_unlocked+0x138/0x150 [drm]
[ 107.676852] drm_gem_object_release_handle+0x5c/0xcc [drm]
[ 107.676886] drm_gem_handle_delete+0x68/0xbc [drm]
[ 107.788743] drm_gem_close_ioctl+0x34/0x40 [drm]
[ 107.793553] drm_ioctl_kernel+0xc0/0x130 [drm]
[ 107.798178] drm_ioctl+0x360/0x4e0 [drm]
[ 107.802277] __arm64_sys_ioctl+0xac/0x104
[ 107.806436] invoke_syscall+0x48/0x104
[ 107.810334] el0_svc_common.constprop.0+0x40/0xe0
[ 107.815209] do_el0_svc+0x1c/0x28
[ 107.818662] el0_svc+0x34/0xec
[ 107.821838] el0t_64_sync_handler+0xa0/0xe4
[ 107.826173] el0t_64_sync+0x198/0x19c
[ 107.829971] ---[ end trace 0000000000000000 ]---
[ 107.834789] ------------[ cut here ]------------
[ 107.839587] refcount_t: underflow; use-after-free.
[ 107.844553] WARNING: CPU: 3 PID: 2040 at lib/refcount.c:28 refcount_warn_saturate+0xf4/0x144
[ 108.052928] CPU: 3 UID: 1000 PID: 2040 Comm: .gnome-shell-wr Tainted: G W 6.17.0-rc7 #1 PREEMPT
[ 108.063491] Tainted: [W]=WARN
[ 108.075627] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[ 108.082808] pc : refcount_warn_saturate+0xf4/0x144
[ 108.087756] lr : refcount_warn_saturate+0xf4/0x144
[ 108.092704] sp : ffff800092a1bb20
[ 108.096141] x29: ffff800092a1bb20 x28: ffff800092a1bce8 x27: ffffbc702dbdbe08
[ 108.103491] x26: 0000000000000008 x25: 0000000000000009 x24: 00000000000000a6
[ 108.110852] x23: ffff00083c72f850 x22: ffff00083c72f868 x21: ffff00087e69f200
[ 108.118222] x20: ffff00087e69f330 x19: ffff00084d157ae0 x18: 0000000000000006
[ 108.125572] x17: 0000000000000000 x16: ffffbc704ba1eda0 x15: ffff800092a1b6ef
[ 108.132925] x14: 000000000000003a x13: 000000000000003a x12: 0000000000000000
[ 108.140280] x11: 00000000000000c0 x10: d2c95932de8ceaa3 x9 : 128386994077d608
[ 108.147631] x8 : ffff000840c0c588 x7 : 0000000002ac3ea0 x6 : 0000000000000002
[ 108.154990] x5 : 0000000435572e2f x4 : 0000000000000002 x3 : 0000000000000010
[ 108.162339] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000840c0b480
[ 108.169697] Call trace:
[ 108.172243] refcount_warn_saturate+0xf4/0x144 (P)
[ 108.177199] msm_gem_free_object+0x25c/0x264 [msm]
[ 108.182167] drm_gem_object_free+0x1c/0x30 [drm]
[ 108.186943] drm_gem_object_handle_put_unlocked+0x138/0x150 [drm]
[ 108.193237] drm_gem_object_release_handle+0x5c/0xcc [drm]
[ 108.198906] drm_gem_handle_delete+0x68/0xbc [drm]
[ 108.203867] drm_gem_close_ioctl+0x34/0x40 [drm]
[ 108.208651] drm_ioctl_kernel+0xc0/0x130 [drm]
[ 108.213248] drm_ioctl+0x360/0x4e0 [drm]
[ 108.217319] __arm64_sys_ioctl+0xac/0x104
[ 108.221464] invoke_syscall+0x48/0x104
[ 108.225343] el0_svc_common.constprop.0+0x40/0xe0
[ 108.230207] do_el0_svc+0x1c/0x28
[ 108.233650] el0_svc+0x34/0xec
[ 108.236817] el0t_64_sync_handler+0xa0/0xe4
[ 108.241143] el0t_64_sync+0x198/0x19c
[ 108.244931] ---[ end trace 0000000000000000 ]---
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/msm: Fix refcnt underflow in error path
2025-09-22 16:32 ` Stephan Gerhold
@ 2025-09-22 16:41 ` Rob Clark
2025-09-22 16:46 ` Stephan Gerhold
0 siblings, 1 reply; 8+ messages in thread
From: Rob Clark @ 2025-09-22 16:41 UTC (permalink / raw)
To: Stephan Gerhold
Cc: dri-devel, linux-arm-msm, freedreno, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, open list
On Mon, Sep 22, 2025 at 9:33 AM Stephan Gerhold
<stephan.gerhold@linaro.org> wrote:
>
> Hi Rob,
>
> On Wed, Jul 23, 2025 at 12:08:49PM -0700, Rob Clark wrote:
> > If we hit an error path in GEM obj creation before msm_gem_new_handle()
> > updates obj->resv to point to the gpuvm resv object, then obj->resv
> > still points to &obj->_resv. In this case we don't want to decrement
> > the refcount of the object being freed (since the refcnt is already
> > zero). This fixes the following splat:
> >
> > ------------[ cut here ]------------
> > refcount_t: underflow; use-after-free.
> > WARNING: CPU: 9 PID: 7013 at lib/refcount.c:28 refcount_warn_saturate+0xf4/0x148
> > Modules linked in: uinput snd_seq_dummy snd_hrtimer aes_ce_ccm snd_soc_wsa884x regmap_sdw q6prm_clocks q6apm_lpass_da>
> > qcom_pil_info i2c_hid drm_kms_helper qcom_common qcom_q6v5 phy_snps_eusb2 qcom_geni_serial drm qcom_sysmon pinctrl_s>
> > CPU: 9 UID: 1000 PID: 7013 Comm: deqp-vk Not tainted 6.16.0-rc4-debug+ #25 PREEMPT(voluntary)
> > Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN53WW 08/02/2024
> > pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> > pc : refcount_warn_saturate+0xf4/0x148
> > lr : refcount_warn_saturate+0xf4/0x148
> > sp : ffff8000a2073920
> > x29: ffff8000a2073920 x28: 0000000000000010 x27: 0000000000000010
> > x26: 0000000000000042 x25: ffff000810e09800 x24: 0000000000000010
> > x23: ffff8000a2073b94 x22: ffff000ddb22de00 x21: ffff000ddb22dc00
> > x20: ffff000ddb22ddf8 x19: ffff0008024934e0 x18: 000000000000000a
> > x17: 0000000000000000 x16: ffff9f8c67d77340 x15: 0000000000000000
> > x14: 00000000ffffffff x13: 2e656572662d7265 x12: 7466612d65737520
> > x11: 3b776f6c66726564 x10: 00000000ffff7fff x9 : ffff9f8c67506c70
> > x8 : ffff9f8c69fa26f0 x7 : 00000000000bffe8 x6 : c0000000ffff7fff
> > x5 : ffff000f53e14548 x4 : ffff6082ea2b2000 x3 : ffff0008b86ab080
> > x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0008b86ab080
> > Call trace:
> > refcount_warn_saturate+0xf4/0x148 (P)
> > msm_gem_free_object+0x248/0x260 [msm]
> > drm_gem_object_free+0x24/0x40 [drm]
> > msm_gem_new+0x1c4/0x1e0 [msm]
> > msm_gem_new_handle+0x3c/0x1a0 [msm]
> > msm_ioctl_gem_new+0x38/0x70 [msm]
> > drm_ioctl_kernel+0xc8/0x138 [drm]
> > drm_ioctl+0x2c8/0x618 [drm]
> > __arm64_sys_ioctl+0xac/0x108
> > invoke_syscall.constprop.0+0x64/0xe8
> > el0_svc_common.constprop.0+0x40/0xe8
> > do_el0_svc+0x24/0x38
> > el0_svc+0x54/0x1d8
> > el0t_64_sync_handler+0x10c/0x138
> > el0t_64_sync+0x19c/0x1a0
> > irq event stamp: 3698694
> > hardirqs last enabled at (3698693): [<ffff9f8c675021dc>] __up_console_sem+0x74/0x90
> > hardirqs last disabled at (3698694): [<ffff9f8c68ce8164>] el1_dbg+0x24/0x90
> > softirqs last enabled at (3697578): [<ffff9f8c6744ec5c>] handle_softirqs+0x454/0x4b0
> > softirqs last disabled at (3697567): [<ffff9f8c67360244>] __do_softirq+0x1c/0x28
> > ---[ end trace 0000000000000000 ]---
> >
> > Fixes: b58e12a66e47 ("drm/msm: Add _NO_SHARE flag")
> > Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> > ---
> > drivers/gpu/drm/msm/msm_gem.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> > index 33d3354c6102..958bac4e2768 100644
> > --- a/drivers/gpu/drm/msm/msm_gem.c
> > +++ b/drivers/gpu/drm/msm/msm_gem.c
> > @@ -1114,10 +1114,12 @@ static void msm_gem_free_object(struct drm_gem_object *obj)
> > put_pages(obj);
> > }
> >
> > - if (msm_obj->flags & MSM_BO_NO_SHARE) {
> > + if (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);
> > }
>
> This patch seems to break something for direct IRIS/video playback using
> dmabuf. I use a simple GStreamer test pipeline for testing IRIS on X1E
> (on GNOME, in case that matters):
>
> $ gst-launch-1.0 filesrc location=bbb_sunflower_2160p_60fps_normal.mp4 \
> ! qtdemux name=d d.video_0 ! h264parse ! v4l2h264dec \
> ! capture-io-mode=dmabuf ! waylandsink
>
> The video plays fine, but if I try to exit (CTRL+C) the display hangs
> for a few seconds and then the console is spammed with pretty much
> exactly the messages that you tried to fix here. If I revert this patch,
> everything is fine again. It feels like your patch does exactly the
> opposite for this use case. :-)
>
> It seems to run into the WARN_ON you added.
Hmm, are we allocating from drm and importing into v4l2, or the other direction?
BR,
-R
> Any ideas?
>
> linux-next should have IRIS support for the Slim 7x if you want to try
> this for yourself. Or alternatively, there is a backport for 6.17-rc7 in
> the Linaro arm64-laptops tree: https://gitlab.com/Linaro/arm64-laptops/linux
>
> You can find the test video here:
> https://download.blender.org/demo/movies/BBB/
>
> Thanks,
> Stephan
>
> [ 107.430721] ------------[ cut here ]------------
> [ 107.435513] WARNING: CPU: 3 PID: 2040 at drivers/gpu/drm/msm/msm_gem.c:1127 msm_gem_free_object+0x1f8/0x264 [msm]
> [ 107.630472] CPU: 3 UID: 1000 PID: 2040 Comm: .gnome-shell-wr Not tainted 6.17.0-rc7 #1 PREEMPT
> [ 107.630482] pstate: 81400005 (Nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> [ 107.630488] pc : msm_gem_free_object+0x1f8/0x264 [msm]
> [ 107.676630] lr : msm_gem_free_object+0x138/0x264 [msm]
> [ 107.676666] sp : ffff800092a1bb30
> [ 107.676668] x29: ffff800092a1bb80 x28: ffff800092a1bce8 x27: ffffbc702dbdbe08
> [ 107.676676] x26: 0000000000000008 x25: 0000000000000009 x24: 00000000000000a6
> [ 107.676682] x23: ffff00083c72f850 x22: ffff00083c72f868 x21: ffff00087e69f200
> [ 107.676689] x20: ffff00087e69f330 x19: ffff00084d157ae0 x18: 0000000000000000
> [ 107.676695] x17: 0000000000000000 x16: ffffbc704bd46b80 x15: 0000ffffd0959540
> [ 107.676701] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
> [ 107.676706] x11: ffffbc702e6cdb48 x10: 0000000000000000 x9 : 000000000000003f
> [ 107.676712] x8 : ffff800092a1ba90 x7 : 0000000000000000 x6 : 0000000000000020
> [ 107.676718] x5 : ffffbc704bd46c40 x4 : fffffdffe102cf60 x3 : 0000000000400032
> [ 107.676724] x2 : 0000000000020000 x1 : ffff00087e6978e8 x0 : ffff00087e6977e8
> [ 107.676731] Call trace:
> [ 107.676733] msm_gem_free_object+0x1f8/0x264 [msm] (P)
> [ 107.676771] drm_gem_object_free+0x1c/0x30 [drm]
> [ 107.676816] drm_gem_object_handle_put_unlocked+0x138/0x150 [drm]
> [ 107.676852] drm_gem_object_release_handle+0x5c/0xcc [drm]
> [ 107.676886] drm_gem_handle_delete+0x68/0xbc [drm]
> [ 107.788743] drm_gem_close_ioctl+0x34/0x40 [drm]
> [ 107.793553] drm_ioctl_kernel+0xc0/0x130 [drm]
> [ 107.798178] drm_ioctl+0x360/0x4e0 [drm]
> [ 107.802277] __arm64_sys_ioctl+0xac/0x104
> [ 107.806436] invoke_syscall+0x48/0x104
> [ 107.810334] el0_svc_common.constprop.0+0x40/0xe0
> [ 107.815209] do_el0_svc+0x1c/0x28
> [ 107.818662] el0_svc+0x34/0xec
> [ 107.821838] el0t_64_sync_handler+0xa0/0xe4
> [ 107.826173] el0t_64_sync+0x198/0x19c
> [ 107.829971] ---[ end trace 0000000000000000 ]---
> [ 107.834789] ------------[ cut here ]------------
> [ 107.839587] refcount_t: underflow; use-after-free.
> [ 107.844553] WARNING: CPU: 3 PID: 2040 at lib/refcount.c:28 refcount_warn_saturate+0xf4/0x144
> [ 108.052928] CPU: 3 UID: 1000 PID: 2040 Comm: .gnome-shell-wr Tainted: G W 6.17.0-rc7 #1 PREEMPT
> [ 108.063491] Tainted: [W]=WARN
> [ 108.075627] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> [ 108.082808] pc : refcount_warn_saturate+0xf4/0x144
> [ 108.087756] lr : refcount_warn_saturate+0xf4/0x144
> [ 108.092704] sp : ffff800092a1bb20
> [ 108.096141] x29: ffff800092a1bb20 x28: ffff800092a1bce8 x27: ffffbc702dbdbe08
> [ 108.103491] x26: 0000000000000008 x25: 0000000000000009 x24: 00000000000000a6
> [ 108.110852] x23: ffff00083c72f850 x22: ffff00083c72f868 x21: ffff00087e69f200
> [ 108.118222] x20: ffff00087e69f330 x19: ffff00084d157ae0 x18: 0000000000000006
> [ 108.125572] x17: 0000000000000000 x16: ffffbc704ba1eda0 x15: ffff800092a1b6ef
> [ 108.132925] x14: 000000000000003a x13: 000000000000003a x12: 0000000000000000
> [ 108.140280] x11: 00000000000000c0 x10: d2c95932de8ceaa3 x9 : 128386994077d608
> [ 108.147631] x8 : ffff000840c0c588 x7 : 0000000002ac3ea0 x6 : 0000000000000002
> [ 108.154990] x5 : 0000000435572e2f x4 : 0000000000000002 x3 : 0000000000000010
> [ 108.162339] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000840c0b480
> [ 108.169697] Call trace:
> [ 108.172243] refcount_warn_saturate+0xf4/0x144 (P)
> [ 108.177199] msm_gem_free_object+0x25c/0x264 [msm]
> [ 108.182167] drm_gem_object_free+0x1c/0x30 [drm]
> [ 108.186943] drm_gem_object_handle_put_unlocked+0x138/0x150 [drm]
> [ 108.193237] drm_gem_object_release_handle+0x5c/0xcc [drm]
> [ 108.198906] drm_gem_handle_delete+0x68/0xbc [drm]
> [ 108.203867] drm_gem_close_ioctl+0x34/0x40 [drm]
> [ 108.208651] drm_ioctl_kernel+0xc0/0x130 [drm]
> [ 108.213248] drm_ioctl+0x360/0x4e0 [drm]
> [ 108.217319] __arm64_sys_ioctl+0xac/0x104
> [ 108.221464] invoke_syscall+0x48/0x104
> [ 108.225343] el0_svc_common.constprop.0+0x40/0xe0
> [ 108.230207] do_el0_svc+0x1c/0x28
> [ 108.233650] el0_svc+0x34/0xec
> [ 108.236817] el0t_64_sync_handler+0xa0/0xe4
> [ 108.241143] el0t_64_sync+0x198/0x19c
> [ 108.244931] ---[ end trace 0000000000000000 ]---
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/msm: Fix refcnt underflow in error path
2025-09-22 16:41 ` Rob Clark
@ 2025-09-22 16:46 ` Stephan Gerhold
2025-09-22 17:42 ` Rob Clark
0 siblings, 1 reply; 8+ messages in thread
From: Stephan Gerhold @ 2025-09-22 16:46 UTC (permalink / raw)
To: Rob Clark
Cc: dri-devel, linux-arm-msm, freedreno, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, open list
On Mon, Sep 22, 2025 at 09:41:07AM -0700, Rob Clark wrote:
> On Mon, Sep 22, 2025 at 9:33 AM Stephan Gerhold
> <stephan.gerhold@linaro.org> wrote:
> > On Wed, Jul 23, 2025 at 12:08:49PM -0700, Rob Clark wrote:
> > > If we hit an error path in GEM obj creation before msm_gem_new_handle()
> > > updates obj->resv to point to the gpuvm resv object, then obj->resv
> > > still points to &obj->_resv. In this case we don't want to decrement
> > > the refcount of the object being freed (since the refcnt is already
> > > zero). This fixes the following splat:
> > >
> > > ------------[ cut here ]------------
> > > refcount_t: underflow; use-after-free.
> > > WARNING: CPU: 9 PID: 7013 at lib/refcount.c:28 refcount_warn_saturate+0xf4/0x148
> > > Modules linked in: uinput snd_seq_dummy snd_hrtimer aes_ce_ccm snd_soc_wsa884x regmap_sdw q6prm_clocks q6apm_lpass_da>
> > > qcom_pil_info i2c_hid drm_kms_helper qcom_common qcom_q6v5 phy_snps_eusb2 qcom_geni_serial drm qcom_sysmon pinctrl_s>
> > > CPU: 9 UID: 1000 PID: 7013 Comm: deqp-vk Not tainted 6.16.0-rc4-debug+ #25 PREEMPT(voluntary)
> > > Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN53WW 08/02/2024
> > > pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> > > pc : refcount_warn_saturate+0xf4/0x148
> > > lr : refcount_warn_saturate+0xf4/0x148
> > > sp : ffff8000a2073920
> > > x29: ffff8000a2073920 x28: 0000000000000010 x27: 0000000000000010
> > > x26: 0000000000000042 x25: ffff000810e09800 x24: 0000000000000010
> > > x23: ffff8000a2073b94 x22: ffff000ddb22de00 x21: ffff000ddb22dc00
> > > x20: ffff000ddb22ddf8 x19: ffff0008024934e0 x18: 000000000000000a
> > > x17: 0000000000000000 x16: ffff9f8c67d77340 x15: 0000000000000000
> > > x14: 00000000ffffffff x13: 2e656572662d7265 x12: 7466612d65737520
> > > x11: 3b776f6c66726564 x10: 00000000ffff7fff x9 : ffff9f8c67506c70
> > > x8 : ffff9f8c69fa26f0 x7 : 00000000000bffe8 x6 : c0000000ffff7fff
> > > x5 : ffff000f53e14548 x4 : ffff6082ea2b2000 x3 : ffff0008b86ab080
> > > x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0008b86ab080
> > > Call trace:
> > > refcount_warn_saturate+0xf4/0x148 (P)
> > > msm_gem_free_object+0x248/0x260 [msm]
> > > drm_gem_object_free+0x24/0x40 [drm]
> > > msm_gem_new+0x1c4/0x1e0 [msm]
> > > msm_gem_new_handle+0x3c/0x1a0 [msm]
> > > msm_ioctl_gem_new+0x38/0x70 [msm]
> > > drm_ioctl_kernel+0xc8/0x138 [drm]
> > > drm_ioctl+0x2c8/0x618 [drm]
> > > __arm64_sys_ioctl+0xac/0x108
> > > invoke_syscall.constprop.0+0x64/0xe8
> > > el0_svc_common.constprop.0+0x40/0xe8
> > > do_el0_svc+0x24/0x38
> > > el0_svc+0x54/0x1d8
> > > el0t_64_sync_handler+0x10c/0x138
> > > el0t_64_sync+0x19c/0x1a0
> > > irq event stamp: 3698694
> > > hardirqs last enabled at (3698693): [<ffff9f8c675021dc>] __up_console_sem+0x74/0x90
> > > hardirqs last disabled at (3698694): [<ffff9f8c68ce8164>] el1_dbg+0x24/0x90
> > > softirqs last enabled at (3697578): [<ffff9f8c6744ec5c>] handle_softirqs+0x454/0x4b0
> > > softirqs last disabled at (3697567): [<ffff9f8c67360244>] __do_softirq+0x1c/0x28
> > > ---[ end trace 0000000000000000 ]---
> > >
> > > Fixes: b58e12a66e47 ("drm/msm: Add _NO_SHARE flag")
> > > Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> > > ---
> > > drivers/gpu/drm/msm/msm_gem.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> > > index 33d3354c6102..958bac4e2768 100644
> > > --- a/drivers/gpu/drm/msm/msm_gem.c
> > > +++ b/drivers/gpu/drm/msm/msm_gem.c
> > > @@ -1114,10 +1114,12 @@ static void msm_gem_free_object(struct drm_gem_object *obj)
> > > put_pages(obj);
> > > }
> > >
> > > - if (msm_obj->flags & MSM_BO_NO_SHARE) {
> > > + if (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);
> > > }
> >
> > This patch seems to break something for direct IRIS/video playback using
> > dmabuf. I use a simple GStreamer test pipeline for testing IRIS on X1E
> > (on GNOME, in case that matters):
> >
> > $ gst-launch-1.0 filesrc location=bbb_sunflower_2160p_60fps_normal.mp4 \
> > ! qtdemux name=d d.video_0 ! h264parse ! v4l2h264dec \
> > ! capture-io-mode=dmabuf ! waylandsink
> >
> > The video plays fine, but if I try to exit (CTRL+C) the display hangs
> > for a few seconds and then the console is spammed with pretty much
> > exactly the messages that you tried to fix here. If I revert this patch,
> > everything is fine again. It feels like your patch does exactly the
> > opposite for this use case. :-)
> >
> > It seems to run into the WARN_ON you added.
>
> Hmm, are we allocating from drm and importing into v4l2, or the other direction?
>
Is there an easy way to check?
I would need to study the code to be sure, you probably know more about
this than I do. I just run this command and it always worked so far
somehow. :-)
Thanks,
Stephan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/msm: Fix refcnt underflow in error path
2025-09-22 16:46 ` Stephan Gerhold
@ 2025-09-22 17:42 ` Rob Clark
2025-09-22 18:49 ` Stephan Gerhold
0 siblings, 1 reply; 8+ messages in thread
From: Rob Clark @ 2025-09-22 17:42 UTC (permalink / raw)
To: Stephan Gerhold
Cc: dri-devel, linux-arm-msm, freedreno, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, open list
On Mon, Sep 22, 2025 at 9:46 AM Stephan Gerhold
<stephan.gerhold@linaro.org> wrote:
>
> On Mon, Sep 22, 2025 at 09:41:07AM -0700, Rob Clark wrote:
> > On Mon, Sep 22, 2025 at 9:33 AM Stephan Gerhold
> > <stephan.gerhold@linaro.org> wrote:
> > > On Wed, Jul 23, 2025 at 12:08:49PM -0700, Rob Clark wrote:
> > > > If we hit an error path in GEM obj creation before msm_gem_new_handle()
> > > > updates obj->resv to point to the gpuvm resv object, then obj->resv
> > > > still points to &obj->_resv. In this case we don't want to decrement
> > > > the refcount of the object being freed (since the refcnt is already
> > > > zero). This fixes the following splat:
> > > >
> > > > ------------[ cut here ]------------
> > > > refcount_t: underflow; use-after-free.
> > > > WARNING: CPU: 9 PID: 7013 at lib/refcount.c:28 refcount_warn_saturate+0xf4/0x148
> > > > Modules linked in: uinput snd_seq_dummy snd_hrtimer aes_ce_ccm snd_soc_wsa884x regmap_sdw q6prm_clocks q6apm_lpass_da>
> > > > qcom_pil_info i2c_hid drm_kms_helper qcom_common qcom_q6v5 phy_snps_eusb2 qcom_geni_serial drm qcom_sysmon pinctrl_s>
> > > > CPU: 9 UID: 1000 PID: 7013 Comm: deqp-vk Not tainted 6.16.0-rc4-debug+ #25 PREEMPT(voluntary)
> > > > Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN53WW 08/02/2024
> > > > pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> > > > pc : refcount_warn_saturate+0xf4/0x148
> > > > lr : refcount_warn_saturate+0xf4/0x148
> > > > sp : ffff8000a2073920
> > > > x29: ffff8000a2073920 x28: 0000000000000010 x27: 0000000000000010
> > > > x26: 0000000000000042 x25: ffff000810e09800 x24: 0000000000000010
> > > > x23: ffff8000a2073b94 x22: ffff000ddb22de00 x21: ffff000ddb22dc00
> > > > x20: ffff000ddb22ddf8 x19: ffff0008024934e0 x18: 000000000000000a
> > > > x17: 0000000000000000 x16: ffff9f8c67d77340 x15: 0000000000000000
> > > > x14: 00000000ffffffff x13: 2e656572662d7265 x12: 7466612d65737520
> > > > x11: 3b776f6c66726564 x10: 00000000ffff7fff x9 : ffff9f8c67506c70
> > > > x8 : ffff9f8c69fa26f0 x7 : 00000000000bffe8 x6 : c0000000ffff7fff
> > > > x5 : ffff000f53e14548 x4 : ffff6082ea2b2000 x3 : ffff0008b86ab080
> > > > x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0008b86ab080
> > > > Call trace:
> > > > refcount_warn_saturate+0xf4/0x148 (P)
> > > > msm_gem_free_object+0x248/0x260 [msm]
> > > > drm_gem_object_free+0x24/0x40 [drm]
> > > > msm_gem_new+0x1c4/0x1e0 [msm]
> > > > msm_gem_new_handle+0x3c/0x1a0 [msm]
> > > > msm_ioctl_gem_new+0x38/0x70 [msm]
> > > > drm_ioctl_kernel+0xc8/0x138 [drm]
> > > > drm_ioctl+0x2c8/0x618 [drm]
> > > > __arm64_sys_ioctl+0xac/0x108
> > > > invoke_syscall.constprop.0+0x64/0xe8
> > > > el0_svc_common.constprop.0+0x40/0xe8
> > > > do_el0_svc+0x24/0x38
> > > > el0_svc+0x54/0x1d8
> > > > el0t_64_sync_handler+0x10c/0x138
> > > > el0t_64_sync+0x19c/0x1a0
> > > > irq event stamp: 3698694
> > > > hardirqs last enabled at (3698693): [<ffff9f8c675021dc>] __up_console_sem+0x74/0x90
> > > > hardirqs last disabled at (3698694): [<ffff9f8c68ce8164>] el1_dbg+0x24/0x90
> > > > softirqs last enabled at (3697578): [<ffff9f8c6744ec5c>] handle_softirqs+0x454/0x4b0
> > > > softirqs last disabled at (3697567): [<ffff9f8c67360244>] __do_softirq+0x1c/0x28
> > > > ---[ end trace 0000000000000000 ]---
> > > >
> > > > Fixes: b58e12a66e47 ("drm/msm: Add _NO_SHARE flag")
> > > > Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> > > > ---
> > > > drivers/gpu/drm/msm/msm_gem.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> > > > index 33d3354c6102..958bac4e2768 100644
> > > > --- a/drivers/gpu/drm/msm/msm_gem.c
> > > > +++ b/drivers/gpu/drm/msm/msm_gem.c
> > > > @@ -1114,10 +1114,12 @@ static void msm_gem_free_object(struct drm_gem_object *obj)
> > > > put_pages(obj);
> > > > }
> > > >
> > > > - if (msm_obj->flags & MSM_BO_NO_SHARE) {
> > > > + if (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);
> > > > }
> > >
> > > This patch seems to break something for direct IRIS/video playback using
> > > dmabuf. I use a simple GStreamer test pipeline for testing IRIS on X1E
> > > (on GNOME, in case that matters):
> > >
> > > $ gst-launch-1.0 filesrc location=bbb_sunflower_2160p_60fps_normal.mp4 \
> > > ! qtdemux name=d d.video_0 ! h264parse ! v4l2h264dec \
> > > ! capture-io-mode=dmabuf ! waylandsink
> > >
> > > The video plays fine, but if I try to exit (CTRL+C) the display hangs
> > > for a few seconds and then the console is spammed with pretty much
> > > exactly the messages that you tried to fix here. If I revert this patch,
> > > everything is fine again. It feels like your patch does exactly the
> > > opposite for this use case. :-)
> > >
> > > It seems to run into the WARN_ON you added.
> >
> > Hmm, are we allocating from drm and importing into v4l2, or the other direction?
> >
>
> Is there an easy way to check?
Maybe strace? But, I think this would help, at least if v4l2 is allocating:
- if (obj->resv != &obj->_resv) {
+ if ((msm_obj->flags & MSM_BO_NO_SHARE) && (obj->resv != &obj->_resv)) {
(sorry about gmail mangling the formatting)
BR,
-R
>
> I would need to study the code to be sure, you probably know more about
> this than I do. I just run this command and it always worked so far
> somehow. :-)
>
> Thanks,
> Stephan
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/msm: Fix refcnt underflow in error path
2025-09-22 17:42 ` Rob Clark
@ 2025-09-22 18:49 ` Stephan Gerhold
0 siblings, 0 replies; 8+ messages in thread
From: Stephan Gerhold @ 2025-09-22 18:49 UTC (permalink / raw)
To: Rob Clark
Cc: dri-devel, linux-arm-msm, freedreno, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, open list
On Mon, Sep 22, 2025 at 10:42:52AM -0700, Rob Clark wrote:
> On Mon, Sep 22, 2025 at 9:46 AM Stephan Gerhold
> <stephan.gerhold@linaro.org> wrote:
> >
> > On Mon, Sep 22, 2025 at 09:41:07AM -0700, Rob Clark wrote:
> > > On Mon, Sep 22, 2025 at 9:33 AM Stephan Gerhold
> > > <stephan.gerhold@linaro.org> wrote:
> > > > On Wed, Jul 23, 2025 at 12:08:49PM -0700, Rob Clark wrote:
> > > > > If we hit an error path in GEM obj creation before msm_gem_new_handle()
> > > > > updates obj->resv to point to the gpuvm resv object, then obj->resv
> > > > > still points to &obj->_resv. In this case we don't want to decrement
> > > > > the refcount of the object being freed (since the refcnt is already
> > > > > zero). This fixes the following splat:
> > > > >
> > > > > ------------[ cut here ]------------
> > > > > refcount_t: underflow; use-after-free.
> > > > > WARNING: CPU: 9 PID: 7013 at lib/refcount.c:28 refcount_warn_saturate+0xf4/0x148
> > > > > Modules linked in: uinput snd_seq_dummy snd_hrtimer aes_ce_ccm snd_soc_wsa884x regmap_sdw q6prm_clocks q6apm_lpass_da>
> > > > > qcom_pil_info i2c_hid drm_kms_helper qcom_common qcom_q6v5 phy_snps_eusb2 qcom_geni_serial drm qcom_sysmon pinctrl_s>
> > > > > CPU: 9 UID: 1000 PID: 7013 Comm: deqp-vk Not tainted 6.16.0-rc4-debug+ #25 PREEMPT(voluntary)
> > > > > Hardware name: LENOVO 83ED/LNVNB161216, BIOS NHCN53WW 08/02/2024
> > > > > pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> > > > > pc : refcount_warn_saturate+0xf4/0x148
> > > > > lr : refcount_warn_saturate+0xf4/0x148
> > > > > sp : ffff8000a2073920
> > > > > x29: ffff8000a2073920 x28: 0000000000000010 x27: 0000000000000010
> > > > > x26: 0000000000000042 x25: ffff000810e09800 x24: 0000000000000010
> > > > > x23: ffff8000a2073b94 x22: ffff000ddb22de00 x21: ffff000ddb22dc00
> > > > > x20: ffff000ddb22ddf8 x19: ffff0008024934e0 x18: 000000000000000a
> > > > > x17: 0000000000000000 x16: ffff9f8c67d77340 x15: 0000000000000000
> > > > > x14: 00000000ffffffff x13: 2e656572662d7265 x12: 7466612d65737520
> > > > > x11: 3b776f6c66726564 x10: 00000000ffff7fff x9 : ffff9f8c67506c70
> > > > > x8 : ffff9f8c69fa26f0 x7 : 00000000000bffe8 x6 : c0000000ffff7fff
> > > > > x5 : ffff000f53e14548 x4 : ffff6082ea2b2000 x3 : ffff0008b86ab080
> > > > > x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0008b86ab080
> > > > > Call trace:
> > > > > refcount_warn_saturate+0xf4/0x148 (P)
> > > > > msm_gem_free_object+0x248/0x260 [msm]
> > > > > drm_gem_object_free+0x24/0x40 [drm]
> > > > > msm_gem_new+0x1c4/0x1e0 [msm]
> > > > > msm_gem_new_handle+0x3c/0x1a0 [msm]
> > > > > msm_ioctl_gem_new+0x38/0x70 [msm]
> > > > > drm_ioctl_kernel+0xc8/0x138 [drm]
> > > > > drm_ioctl+0x2c8/0x618 [drm]
> > > > > __arm64_sys_ioctl+0xac/0x108
> > > > > invoke_syscall.constprop.0+0x64/0xe8
> > > > > el0_svc_common.constprop.0+0x40/0xe8
> > > > > do_el0_svc+0x24/0x38
> > > > > el0_svc+0x54/0x1d8
> > > > > el0t_64_sync_handler+0x10c/0x138
> > > > > el0t_64_sync+0x19c/0x1a0
> > > > > irq event stamp: 3698694
> > > > > hardirqs last enabled at (3698693): [<ffff9f8c675021dc>] __up_console_sem+0x74/0x90
> > > > > hardirqs last disabled at (3698694): [<ffff9f8c68ce8164>] el1_dbg+0x24/0x90
> > > > > softirqs last enabled at (3697578): [<ffff9f8c6744ec5c>] handle_softirqs+0x454/0x4b0
> > > > > softirqs last disabled at (3697567): [<ffff9f8c67360244>] __do_softirq+0x1c/0x28
> > > > > ---[ end trace 0000000000000000 ]---
> > > > >
> > > > > Fixes: b58e12a66e47 ("drm/msm: Add _NO_SHARE flag")
> > > > > Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
> > > > > ---
> > > > > drivers/gpu/drm/msm/msm_gem.c | 4 +++-
> > > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> > > > > index 33d3354c6102..958bac4e2768 100644
> > > > > --- a/drivers/gpu/drm/msm/msm_gem.c
> > > > > +++ b/drivers/gpu/drm/msm/msm_gem.c
> > > > > @@ -1114,10 +1114,12 @@ static void msm_gem_free_object(struct drm_gem_object *obj)
> > > > > put_pages(obj);
> > > > > }
> > > > >
> > > > > - if (msm_obj->flags & MSM_BO_NO_SHARE) {
> > > > > + if (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);
> > > > > }
> > > >
> > > > This patch seems to break something for direct IRIS/video playback using
> > > > dmabuf. I use a simple GStreamer test pipeline for testing IRIS on X1E
> > > > (on GNOME, in case that matters):
> > > >
> > > > $ gst-launch-1.0 filesrc location=bbb_sunflower_2160p_60fps_normal.mp4 \
> > > > ! qtdemux name=d d.video_0 ! h264parse ! v4l2h264dec \
> > > > ! capture-io-mode=dmabuf ! waylandsink
> > > >
> > > > The video plays fine, but if I try to exit (CTRL+C) the display hangs
> > > > for a few seconds and then the console is spammed with pretty much
> > > > exactly the messages that you tried to fix here. If I revert this patch,
> > > > everything is fine again. It feels like your patch does exactly the
> > > > opposite for this use case. :-)
> > > >
> > > > It seems to run into the WARN_ON you added.
> > >
> > > Hmm, are we allocating from drm and importing into v4l2, or the other direction?
> > >
> >
> > Is there an easy way to check?
>
> Maybe strace? But, I think this would help, at least if v4l2 is allocating:
>
I would indeed guess that v4l2 is allocating in this case. I didn't end
up checking with strace, because your proposed change
> - if (obj->resv != &obj->_resv) {
> + if ((msm_obj->flags & MSM_BO_NO_SHARE) && (obj->resv != &obj->_resv)) {
>
fixes the issue. Thanks! If this makes sense to you, could you submit a
patch?
Thanks,
Stephan
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-09-22 18:49 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-23 19:08 [PATCH 0/2] drm/msm: Error path fixes Rob Clark
2025-07-23 19:08 ` [PATCH 1/2] drm/msm: Fix refcnt underflow in error path Rob Clark
2025-09-22 16:32 ` Stephan Gerhold
2025-09-22 16:41 ` Rob Clark
2025-09-22 16:46 ` Stephan Gerhold
2025-09-22 17:42 ` Rob Clark
2025-09-22 18:49 ` Stephan Gerhold
2025-07-23 19:08 ` [PATCH 2/2] drm/msm: Fix submit error path cleanup Rob Clark
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox