linux-kernel - Re: [PATCH 5/6] powerpc/livepatch: Add livepatch stack to struct thread

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]

Message-ID: <56FA1697.8090800@gmail.com>
Date:	Tue, 29 Mar 2016 16:45:59 +1100
From:	Balbir Singh <bsingharora@...il.com>
To:	Michael Ellerman <mpe@...erman.id.au>, linuxppc-dev@...abs.org
Cc:	duwe@....de, linux-kernel@...r.kernel.org, rostedt@...dmis.org,
	kamalesh@...ux.vnet.ibm.com, pmladek@...e.com, jeyu@...hat.com,
	jkosina@...e.cz, live-patching@...r.kernel.org, mbenes@...e.cz
Subject: Re: [PATCH 5/6] powerpc/livepatch: Add livepatch stack to struct
 thread_info


>> At this point ti->livepatch_sp points to the next CPUs thread_info for softirq_ctx?
> Sorry I'm not sure what you mean.
>
> None of this relates to the current CPUs thread info.
Oh! I meant that klp_init_thread_info points to the end of (struct thread_info {} + 1) in the stack of the thread/task, but with the irq_contexts they are a separate array and not on stack
>>> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
>>> index 3807fb05b6de..1b6cabb8e715 100644
>>> --- a/arch/powerpc/kernel/setup_64.c
>>> +++ b/arch/powerpc/kernel/setup_64.c
>>> @@ -69,6 +69,7 @@
>>>  #include <asm/kvm_ppc.h>
>>>  #include <asm/hugetlb.h>
>>>  #include <asm/epapr_hcalls.h>
>>> +#include <asm/livepatch.h>
>>>  
>>>  #ifdef DEBUG
>>>  #define DBG(fmt...) udbg_printf(fmt)
>>> @@ -667,16 +668,16 @@ static void __init emergency_stack_init(void)
>>>  	limit = min(safe_stack_limit(), ppc64_rma_size);
>>>  
>>>  	for_each_possible_cpu(i) {
>>> -		unsigned long sp;
>>> -		sp  = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
>>> -		sp += THREAD_SIZE;
>>> -		paca[i].emergency_sp = __va(sp);
>>> +		struct thread_info *ti;
>>> +		ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit));
>>> +		klp_init_thread_info(ti);
>>> +		paca[i].emergency_sp = (void *)ti + THREAD_SIZE;
>>  
>> Does emergency_sp still end up 128 byte aligned after this?
> It should end up THREAD_SIZE aligned as before, due to the memblock_alloc_base().
Yep.. I missed it.. so where is the space for ti? The stack is going to go grow downwards from ti+THREAD_SIZE
>>>  #ifdef CONFIG_PPC_BOOK3S_64
>>>  		/* emergency stack for machine check exception handling. */
>>> -		sp  = memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit);
>>> -		sp += THREAD_SIZE;
>>> -		paca[i].mc_emergency_sp = __va(sp);
>>> +		ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit));
>>> +		klp_init_thread_info(ti);
>> Do we care about live-patching in this context? Are we mixing per-thread and per-cpu contexts?
> Well we probably don't want to be doing live patching when we're on the
> emergency stacks. But we have no control over whether that happens so we have
> to support it.
>

OK.. I was wondering if the code will even work.. I wonder if the ftrace data structures will work in real mode, including the hash/etc.

Balbir