linux-kernel - Re: [PATCH] powerpc: mpc5200: Remove VLA usage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 29 Jun 2018 23:02:15 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Kees Cook <keescook@...omium.org>
Cc:     Anatolij Gustschin <agust@...x.de>,
        Paul Mackerras <paulus@...ba.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] powerpc: mpc5200: Remove VLA usage

On Fri, Jun 29, 2018 at 8:53 PM, Kees Cook <keescook@...omium.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=qPXydAacU1RqZWA@mail.gmail.com
>
> Signed-off-by: Kees Cook <keescook@...omium.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.

Reviewed-by: Arnd Bergmann <arnd@...db.de>

> ---
>  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);