On Fri, Jun 29, 2018 at 2:02 PM, Arnd Bergmann <a...@arndb.de> wrote: > On Fri, Jun 29, 2018 at 8:53 PM, Kees Cook <keesc...@chromium.org> wrote: >> In the quest to remove all stack VLA usage from the kernel[1], this >> switches to using a stack size large enough for the saved routine and >> adds a sanity check. >> >> [1] >> https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qpxydaacu1rq...@mail.gmail.com >> >> Signed-off-by: Kees Cook <keesc...@chromium.org> > > This seems particularly nice, not only avoids it the dynamic stack > allocation, it > also makes sure the new 0x500 handler doesn't overflow into the 0x600 > exception handler. > > It would help to explain how you arrived at that '256 byte' number in > the changelog though.
Honestly, I just counted instructions, multiplied by 8 and rounded up to the next nearest power of 2, and the result felt right for giving some level of flexibility for code growth before tripping the WARN. :P I'm happy to adjust, of course. :) -Kees > Reviewed-by: Arnd Bergmann <a...@arndb.de> Thanks! -Kees > >> --- >> arch/powerpc/platforms/52xx/mpc52xx_pm.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_pm.c >> b/arch/powerpc/platforms/52xx/mpc52xx_pm.c >> index 31d3515672f3..b23da85fa73c 100644 >> --- a/arch/powerpc/platforms/52xx/mpc52xx_pm.c >> +++ b/arch/powerpc/platforms/52xx/mpc52xx_pm.c >> @@ -117,7 +117,10 @@ int mpc52xx_pm_enter(suspend_state_t state) >> u32 intr_main_mask; >> void __iomem * irq_0x500 = (void __iomem *)CONFIG_KERNEL_START + >> 0x500; >> unsigned long irq_0x500_stop = (unsigned long)irq_0x500 + >> mpc52xx_ds_cached_size; >> - char saved_0x500[mpc52xx_ds_cached_size]; >> + char saved_0x500[256]; >> + >> + if (WARN_ON(mpc52xx_ds_cached_size > sizeof(saved_0x500))) >> + return -ENOMEM; >> >> /* disable all interrupts in PIC */ >> intr_main_mask = in_be32(&intr->main_mask); -- Kees Cook Pixel Security
