[PATCH 5/5] ARM: spectre-v2: per-CPU vtables to work around big.Little systems

[julien.thierry@arm.com (Julien Thierry)]

On 20/09/18 10:21, Russell King - ARM Linux wrote:
> On Thu, Sep 20, 2018 at 10:04:44AM +0100, Julien Thierry wrote:
>> Hi Russell,
>>
>> On 19/09/18 10:49, Russell King wrote:
>>> In big.Little systems, some CPUs require the Spectre workarounds in
>>> paths such as the context switch, but other CPUs do not.  In order
>>> to handle these differences, we need per-CPU vtables.
>>>
>>> We are unable to use the kernel's per-CPU variables to support this
>>> as per-CPU is not initialised at times when we need access to the
>>> vtables, so we have to use an array indexed by logical CPU number.
>>>
>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>>> ---
>>>   arch/arm/include/asm/proc-fns.h | 14 ++++++++++++++
>>>   arch/arm/kernel/setup.c         |  5 +++++
>>>   arch/arm/kernel/smp.c           | 31 +++++++++++++++++++++++++++++++
>>>   arch/arm/mm/proc-v7-bugs.c      | 17 ++---------------
>>>   4 files changed, 52 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/proc-fns.h b/arch/arm/include/asm/proc-fns.h
>>> index 571a1346245b..ddbb25cb8968 100644
>>> --- a/arch/arm/include/asm/proc-fns.h
>>> +++ b/arch/arm/include/asm/proc-fns.h
>>> @@ -104,11 +104,25 @@ extern void cpu_do_resume(void *);
>>>   #else
>>>   extern struct processor processor;
>>> +#if defined(CONFIG_BIG_LITTLE) && defined(CONFIG_HARDEN_BRANCH_PREDICTOR)
>>> +#include <linux/smp.h>
>>> +/*
>>> + * This can't be a per-cpu variable because we need to access it before
>>> + * per-cpu has been initialised.
>>> + */
>>> +extern struct processor *cpu_vtable[];
>>> +#define PROC_VTABLE(f)			cpu_vtable[smp_processor_id()]->f
>>> +static inline void init_proc_vtable(const struct processor *p)
>>> +{
>>> +	*cpu_vtable[smp_processor_id()] = *p;
>>> +}
>>
>> As you stated, 32-bit ARM can only have up to 8 logical CPUs. So, do we save
>> much by having an added level of indirection? (i.e. having a table of
>> pointers instead of directly having a table of struct processor)
> 
> Yes we do - in a word, security.
> 
> If we made this a table of struct processor, because it is written to
> after boot, it would need to be writable for the lifetime of the kernel.
> Since it is in the kernel's static data section, it will be at a well-
> known offset, which means it's a target for attackers to change the
> function pointers.
> 
> Exactly this was true of the 'processor' variable, which gained a
> __ro_after_init annotation a while back - and going to a table of
> struct processor would mean that we end up back in that situation.
> 
> Having a table of pointers to struct processor means there is an
> additional level of complexity for attackers to get around - they would
> first need to copy an existing table, change its contents, and then
> change the appropriate CPU's cpu_vtable pointer to point to their table.
> Having a table of struct processor just means an attacker would only
> need to poke one 32-bit word in the array for the appropriate CPU to
> gain control.
> 
> So yes, this is slightly more complex, but it's that way to avoid
> undoing the benefits that have previously been gained through
> facilities such as read-only data after kernel initialisation.
> 

I see, thanks for explaining. A very good reason for it then :) .

Reviewed-by: Julien Thierry <julien.thierry@arm.com>

Thanks,

-- 
Julien Thierry