linux-kernel - Re: [PATCH] drm:amd:amdgpu: Fix missing bo unlock in failure 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] [day] [month] [year] [list]

Date:   Mon, 24 Apr 2023 11:01:35 -0400
From:   Alex Deucher <alexdeucher@...il.com>
To:     Christian König <christian.koenig@....com>
Cc:     Sukrut Bellary <sukrut.bellary@...ux.com>, daniel@...ll.ch,
        airlied@...il.com, sumit.semwal@...aro.org, Hawking.Zhang@....com,
        Julia.Lawall@...ia.fr, dri-devel@...ts.freedesktop.org,
        Xinhui.Pan@....com, linux-kernel@...r.kernel.org,
        amd-gfx@...ts.freedesktop.org, linaro-mm-sig@...ts.linaro.org,
        alexander.deucher@....com, linux-media@...r.kernel.org
Subject: Re: [PATCH] drm:amd:amdgpu: Fix missing bo unlock in failure path

On Mon, Apr 24, 2023 at 3:07 AM Christian König
<christian.koenig@....com> wrote:
>
> Am 24.04.23 um 07:59 schrieb Sukrut Bellary:
> > smatch warning - inconsistent handling of buffer object reserve
> > and unreserve.
> >
> > Signed-off-by: Sukrut Bellary <sukrut.bellary@...ux.com>
>
> For now that patch is Reviewed-by: Christian König
> <christian.koenig@....com>.

Applied.  Thanks.

>
> But for the record mapping/unmapping the MQD like this is a very bad
> idea in the first place.
>
> We could need to shuffle memory around for that during resume and that
> is not something we really want to do.

We should probably just keep the MQDs mapped.  On suspend we need to
save out the MQD state so it can be restored on resume when the MQDs
are in vram.

Alex

>
> Christian.
>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index 278416acf060..5de44d7e92de 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -4686,8 +4686,10 @@ static int gfx_v8_0_kiq_resume(struct amdgpu_device *adev)
> >               return r;
> >
> >       r = amdgpu_bo_kmap(ring->mqd_obj, &ring->mqd_ptr);
> > -     if (unlikely(r != 0))
> > +     if (unlikely(r != 0)) {
> > +             amdgpu_bo_unreserve(ring->mqd_obj);
> >               return r;
> > +     }
> >
> >       gfx_v8_0_kiq_init_queue(ring);
> >       amdgpu_bo_kunmap(ring->mqd_obj);
>