[PATCH] arm64: cache: Lower ARCH_DMA_MINALIGN to 64 (L1_CACHE_BYTES)

[Maxime Ripard]

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 at kernel.org> 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.
> 
> This code was introduced by Maxime Ripard in commit 9ec03d7f1ed3
>  ("drm/vc4: kms: Wait on previous FIFO users before a commit").

Awesome, I tried to find out that bug a few weeks ago but couldn't
reproduce the KASAN spat. You're right, it should be channel here
instead of i. Since you did the whole work, do you want to send the
patch?

maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20210607/9814c20a/attachment-0001.sig>