lkml.org 
[lkml]   [2018]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 2/2] powerpc/32: add stack protector support
From
Date


Le 19/09/2018 à 15:26, Segher Boessenkool a écrit :
> On Wed, Sep 19, 2018 at 11:14:45AM +0000, Christophe Leroy wrote:
>> --- a/arch/powerpc/Makefile
>> +++ b/arch/powerpc/Makefile
>> @@ -112,6 +112,10 @@ KBUILD_LDFLAGS += -m elf$(BITS)$(LDEMULATION)
>> KBUILD_ARFLAGS += --target=elf$(BITS)-$(GNUTARGET)
>> endif
>>
>> +cflags-$(CONFIG_STACKPROTECTOR) += -mstack-protector-guard=tls
>> +cflags-$(CONFIG_STACKPROTECTOR) += -mstack-protector-guard-reg=r2
>> +cflags-$(CONFIG_STACKPROTECTOR) += -mstack-protector-guard-offset=4
>
> This last line is only correct if !CONFIG_THREAD_INFO_IN_TASK; is that
> always true? Add an assert somewhere maybe?

At the time being powerpc doesn't select CONFIG_THREAD_INFO_IN_TASK.

A BUILD_BUG_ON() is added in stackprotector.h

>
>> + /*
>> + * The stack_canary must be located at the offset given to
>> + * -mstack-protector-guard-offset in the Makefile
>> + */
>> + BUILD_BUG_ON(offsetof(struct task_struct, stack_canary) != sizeof(long));
>
> Well this will help :-)
>
> It looks like it will be easy to enable on 64 bit as well.

Will it ? It seems that PPC64 doesn't have r2 pointing to current task
struct, but instead it has r13 pointing to the paca struct. Which means
we should add a canary in the paca struct, and populate it at task
switch from current->stack_canary. Or am I missing something ?

>
>> + /* Try to get a semi random initial value. */
>> + get_random_bytes(&canary, sizeof(canary));
>> + canary ^= mftb();
>> + canary ^= LINUX_VERSION_CODE;
>
> These last two lines are useless (or worse, they may give people the idea
> that they are not!)

Well, the last line is in all arches except x86
The mftb() was suggested by Michael to add some entropy.
x86 does the same sort of thing with their rdtsc()

>
> You should use wait_for_random_bytes I think.

On the 8xx, it takes several minutes before crnd_is_ready(), while
boot_init_stack_canary() is called quite early in start_kernel()

Christophe

\
 
 \ /
  Last update: 2018-09-19 16:23    [W:0.045 / U:1.924 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site