Re: FYI: imx-sdma firmware is not compatible with SLUB slab allocator

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Thu, Sep 12, 2019 at 02:19:51PM +0000, Robin Gong wrote:
> > -----Original Message-----
> On 2019/9/12 20:12 Jurgen Lambrecht <J.Lambrecht@TELEVIC.com> wrote:
> > 
> > On 9/12/19 11:45 AM, Jurgen Lambrecht wrote:
> > > CAUTION: This Email originated from outside Televic. Do not click links or
> > open attachments unless you recognize the sender and know the content is
> > safe.
> > >
> > >
> > > On 9/12/19 4:06 AM, Robin Gong wrote:
> > >>> (this looked the most interesting commit)
> > >> I identified this issue which caused by
> > >> SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V3 (41)exceed the structure
> > >> sdma_script_start_addrs(40) so that illegal memory touch, such as
> > >> slob block header, thus kernel trap into while() loop forever in slob_free().
> > Please see the below code piece in sdma_add_scripts().
> > >>           for (i = 0; i < sdma->script_number; i++)
> > >>                   if (addr_arr[i] > 0)
> > >>                           saddr_arr[i] = addr_arr[i]; That issue was
> > >> brought by commit a572460be9cf (dmaengine: imx-sdma:
> > >> Add support for version 3 firmware) because the
> > >> SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V3
> > >> (38->41 3 scripts added) not align with script number added in
> > >> sdma_script_start_addrs(2 scripts). Please have a try with the below
> > >> patch:
> > >> diff --git a/include/linux/platform_data/dma-imx-sdma.h
> > >> b/include/linux/platform_data/dma-imx-sdma.h
> > >> index 6eaa53c..30e676b 100644
> > >> --- a/include/linux/platform_data/dma-imx-sdma.h
> > >> +++ b/include/linux/platform_data/dma-imx-sdma.h
> > >> @@ -51,7 +51,10 @@ struct sdma_script_start_addrs {
> > >>           /* End of v2 array */
> > >>           s32 zcanfd_2_mcu_addr;
> > >>           s32 zqspi_2_mcu_addr;
> > >> +       s32 mcu_2_ecspi_addr;
> > >>           /* End of v3 array */
> > >> +       s32 mcu_2_zqspi_addr;
> > >> +       /* End of v4 array */
> > >>    };
> > >>
> > > Yes, this patch solves it! I can now use SLOB slab allocator. I tried
> > > several reboots and power cycles.
> > > I tried with different dts (without earlycon, without sdma on uart and
> > > ecspi).
> > > I did not try other kernels, only 4.19.66+fscl with our patches and
> > > sdma
> > > v3.5 built-in.
> > I tried again v5.3 rc6 with SLOB, but it still booted OK, then took latest v5.3 rc8
> > from mainline, and it also boots OK - tried several times also with power cycle.
> > 
> > Then I added your patch, and it "still" boots :-). So OK:
> > 
> > Linux imx6ul-33927318 5.3.0-rc8-dirty #3 PREEMPT Thu Sep 12 13:54:25 CEST
> > 2019 armv7l GNU/Linux
> Thanks for your test on v5.3. Yes, that potential memory corrupt only happen
> on one word, most time it may hid well during kernel bootup, and it's so luck
> for us that your 'SLOB + firmware built in' case could expose it :).Thanks for
> your report, I'll post a formal patch for review later. 

It sounds like this code is very fragile, and it seems like this mistake
could easily happen again in the future.

How about ensuring that sdma->script_number * sizeof(u32) <
sizeof(struct sdma_script_start_addrs), since sdma_add_scripts() has
no protection against overrunning the structure size.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel