Re: [PATCH v4 1/2] powerpc/32: add stack protector support
From: Segher Boessenkool
Date: Thu Sep 27 2018 - 03:46:20 EST
On Thu, Sep 27, 2018 at 08:20:00AM +0200, Christophe LEROY wrote:
> Le 26/09/2018 à 21:16, Segher Boessenkool a écrit :
> >On Wed, Sep 26, 2018 at 11:40:38AM +0000, Christophe Leroy wrote:
> >>+static __always_inline void boot_init_stack_canary(void)
> >>+{
> >>+ unsigned long canary;
> >>+
> >>+ /* Try to get a semi random initial value. */
> >>+ get_random_bytes(&canary, sizeof(canary));
> >>+ canary ^= mftb();
> >>+ canary ^= LINUX_VERSION_CODE;
> >>+
> >>+ current->stack_canary = canary;
> >>+}
> >
> >I still think you should wait until there is entropy available. You
> >haven't answered my questions about that (or I didn't see them): what
> >does the kernel do in other similar cases?
> >
> >Looks great otherwise!
>
> What do you mean by 'other similar cases' ? All arches have similar
> boot_init_stack_canary().
Yes, those, and other things that want entropy early.
> x86 uses rdtsc() which is equivalent to our
> mftb(). Most arches xor it with LINUX_VERSION_CODE.
>
> The issue is that it is called very early in start_kernel(), however
> they try to set some entropy anyway:
>
> boot_cpu_init();
> page_address_init();
> pr_notice("%s", linux_banner);
> setup_arch(&command_line);
> /*
> * Set up the the initial canary and entropy after arch
> * and after adding latent and command line entropy.
> */
> add_latent_entropy();
> add_device_randomness(command_line, strlen(command_line));
> boot_init_stack_canary();
>
> Apparently, it is too early for calling wait_for_random_bytes(), see below.
Hrm. Too early to call wait_event_interruptible? From there it went
into schedule(), which blew up. Well you say we have only one context
at this point, so that is not too surprising then :-)
> However this is the canary for initial startup only. Only idle() still
> uses this canary once the system is running. A new canary is set for any
> new forked task.
Ah, that makes things a lot better! Do those new tasks get a canary
from something with sufficient entropy though?
> Maybe should the idle canary be updated later once there is more entropy
That is tricky to do, but sure, if you can, that should help.
> ? Today there is a new call to boot_init_stack_canary() in
> cpu_startup_entry(), but it is enclosed inside #ifdef CONFIG_X86.
It needs to know the details of how ssp works on each platform.
Segher