On Mon, Jun 07, 2021 at 03:57:41PM +0200, Arnd Bergmann wrote:
> On Mon, Jun 7, 2021 at 3:39 PM Will Deacon <will@xxxxxxxxxx> wrote:
> >
> > [Adding VC4 folks -- please see the KASAN splat below!]
> >
> > Background here is that reducing ARCH_DMA_MINALIGN to 64 on arm64 (queued in
> > -next) is causing vc4 to hang on Rpi3b due to a probable driver bug.
>
> The great news for the patch that caused it is that this has nothing to
> do with DMA alignment.
>
> > On Mon, Jun 07, 2021 at 02:08:59PM +0100, Mark Rutland wrote:
> > > On Mon, Jun 07, 2021 at 01:01:18PM +0100, Mark Rutland wrote:
> > > > On Mon, Jun 07, 2021 at 11:58:32AM +0200, Marek Szyprowski wrote:
>
> > > [ 3.728042] BUG: KASAN: slab-out-of-bounds in vc4_atomic_commit_tail+0x1cc/0x910
> > > [ 3.728123] Read of size 8 at addr ffff000007360440 by task kworker/u8:0/7
>
> This is offset 0x40 into struct vc4_hvs_state, which is the
> 'pending_commit' pointer
> for the array index 4, i.e. one after the end of the structure.
>
> > > [ 3.728495] kasan_report+0x1dc/0x240
> > > [ 3.728529] __asan_load8+0x98/0xd4
> > > [ 3.728565] vc4_atomic_commit_tail+0x1cc/0x910
>
> It seems to be this loop:
>
> for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) {
> struct vc4_crtc_state *vc4_crtc_state =
> to_vc4_crtc_state(old_crtc_state);
> unsigned int channel = vc4_crtc_state->assigned_channel;
> int ret;
>
> if (channel == VC4_HVS_CHANNEL_DISABLED)
> continue;
>
> if (!old_hvs_state->fifo_state[channel].in_use)
> continue;
>
> ret =
> drm_crtc_commit_wait(old_hvs_state->fifo_state[i].pending_commit);
> if (ret)
> drm_err(dev, "Timed out waiting for commit\n");
> }
>
> I notice that it checks index 'fifos_state[channel].in_use', but then
> uses a different index 'i' for looking at the 'pending_commit' field
> beyond the end of the array.
FWIW, with that drm_crtc_commit_wait() call changed to:
| ret = drm_crtc_commit_wait(old_hvs_state->fifo_state[channel].pending_commit);
... I no longer see a KASAN splat, and I no longer see a hang with
ARCH_DMA_MINALIGN reduced to 64.
Thanks,
Mark.
>
> This code was introduced by Maxime Ripard in commit 9ec03d7f1ed3
> ("drm/vc4: kms: Wait on previous FIFO users before a commit").
>
> Arnd
