From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.free-electrons.com ([62.4.15.54]:43992 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754051AbeAQUcd (ORCPT ); Wed, 17 Jan 2018 15:32:33 -0500 Date: Wed, 17 Jan 2018 21:32:30 +0100 From: Boris Brezillon To: Eric Anholt Cc: David Airlie , Daniel Vetter , dri-devel@lists.freedesktop.org, stable@vger.kernel.org Subject: Re: [PATCH] drm/vc4: Fix NULL pointer dereference in vc4_save_hang_state() Message-ID: <20180117213230.53e6a475@bbrezillon> In-Reply-To: <877esgfe2k.fsf@anholt.net> References: <20180110151825.23477-1-boris.brezillon@free-electrons.com> <877esgfe2k.fsf@anholt.net> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: On Wed, 17 Jan 2018 12:03:15 -0800 Eric Anholt wrote: > Boris Brezillon 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: > > Signed-off-by: Boris Brezillon > > --- > > 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? Sure. > > 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 managed to trigger the NULL pointer dereference while debugging the perfmon stuff, but it's fixed now, so I don't have a way to easily force a reset. > 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?