From: Daniel Vetter <daniel@ffwll.ch>
To: Eric Anholt <eric@anholt.net>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>,
David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
dri-devel@lists.freedesktop.org, stable@vger.kernel.org
Subject: Re: [PATCH] drm/vc4: Fix NULL pointer dereference in vc4_save_hang_state()
Date: Wed, 17 Jan 2018 22:29:05 +0100 [thread overview]
Message-ID: <20180117212904.GA2759@phenom.ffwll.local> (raw)
In-Reply-To: <877esgfe2k.fsf@anholt.net>
On Wed, Jan 17, 2018 at 12:03:15PM -0800, Eric Anholt wrote:
> Boris Brezillon <boris.brezillon@free-electrons.com> writes:
>
> > When saving BOs in the hang state we skip one entry of the
> > kernel_state->bo[] array, thus leaving it to NULL. This leads to a NULL
> > pointer dereference when, later in this function, we iterate over all
> > BOs to check their ->madv state.
> >
> > Fixes: ca26d28bbaa3 ("drm/vc4: improve throughput by pipelining binning and rendering jobs")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> > drivers/gpu/drm/vc4/vc4_gem.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
> > index 6c32c89a83a9..19ac7fe0e5db 100644
> > --- a/drivers/gpu/drm/vc4/vc4_gem.c
> > +++ b/drivers/gpu/drm/vc4/vc4_gem.c
> > @@ -208,7 +208,7 @@ vc4_save_hang_state(struct drm_device *dev)
> > kernel_state->bo[j + prev_idx] = &bo->base.base;
> > j++;
> > }
> > - prev_idx = j + 1;
> > + prev_idx = j;
>
> Could we replace the whole "[j + prev_idx]" with a "[k++]" and maybe a
> WARN_ON_ONCE(k != state->bo_count) at the end?
>
> I really need to figure out if I can come up with a way to make IGT
> cases for GPU hangs on vc4, despite the validation. I found a bug in
> GPU reset due to BCL hangs when doing vc5, but I don't have a testcase.
> Maybe some submit flags that overwrite the BCL or RCL to do an infinite
> loop?
What we currently do for i915 is an endless chain of batches (since no
command parser we can get away with that). Previously we did a special
debugfs mode which blocked out updating the ring head (but left all the
other command submission handling in place). Except for the very minor
change nothing needed to be adjusted in the kernel, and from the kernel's
pov it very much looked like the gpu simply died.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
prev parent reply other threads:[~2018-01-17 21:29 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-10 15:18 [PATCH] drm/vc4: Fix NULL pointer dereference in vc4_save_hang_state() Boris Brezillon
2018-01-17 20:03 ` Eric Anholt
2018-01-17 20:32 ` Boris Brezillon
2018-01-17 21:29 ` Daniel Vetter [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180117212904.GA2759@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=airlied@linux.ie \
--cc=boris.brezillon@free-electrons.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=eric@anholt.net \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox