linux-kernel - Re: [PATCH 1/2] drm/msm: Fix refcnt underflow in error path

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] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACSVV015U3=KpZqxzH7ySO=ok8k-0ff_-Yzc33SK_Smj1c-4-A@mail.gmail.com>
Date: Mon, 22 Sep 2025 10:42:52 -0700
From: Rob Clark <rob.clark@....qualcomm.com>
To: Stephan Gerhold <stephan.gerhold@...aro.org>
Cc: dri-devel@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
        freedreno@...ts.freedesktop.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>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] drm/msm: Fix refcnt underflow in error path

On Mon, Sep 22, 2025 at 9:46 AM Stephan Gerhold
<stephan.gerhold@...aro.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@...aro.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@....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