lkml.org 
[lkml]   [2018]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH v5 3/9] powerpc: Prepare for moving thread_info into task_struct
From
Date


On 10/06/2018 12:40 PM, Michael Ellerman wrote:
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>
>> diff --git a/arch/powerpc/include/asm/livepatch.h b/arch/powerpc/include/asm/livepatch.h
>> index 47a03b9b528b..818451bf629c 100644
>> --- a/arch/powerpc/include/asm/livepatch.h
>> +++ b/arch/powerpc/include/asm/livepatch.h
>> @@ -49,7 +49,7 @@ static inline void klp_init_thread_info(struct thread_info *ti)
>> ti->livepatch_sp = (unsigned long *)(ti + 1) + 1;
>
> We need to do something about this.

Oops I missed that one.

>
> Currently the thread_info sits at the low address of the stack, and we
> use the space immediately above that as a miniature upward growing stack
> for livepatching.
>
> If we keep the livepatch_sp in the thread_info then we need to
> initialise it somewhere when the task starts running on a stack. And I
> don't know how that works if we end up running on the emergency stack
> for example.
>
> So I'm not sure that makes much sense.
>
> Instead we might need to keep the livepatch_sp on the stack page at the
> base, where thread_info currently lives.
>
> That obviously sucks because you can still overflow into it and trash
> it, but it's no worse than what we have now for livepatching.
>
> Need to think about it some more.
>

I think for that serie we can leave with it in the stack, as it won't be
worst than before. Then in a future patch we can change it. I'll open an
issue for that on github.

Then for now, I'll add the following change in this patch.

diff --git a/arch/powerpc/include/asm/livepatch.h
b/arch/powerpc/include/asm/livepatch.h
index 47a03b9b528b..8a81d10ccc82 100644
--- a/arch/powerpc/include/asm/livepatch.h
+++ b/arch/powerpc/include/asm/livepatch.h
@@ -43,13 +43,14 @@ static inline unsigned long
klp_get_ftrace_location(unsigned long faddr)
return ftrace_location_range(faddr, faddr + 16);
}

-static inline void klp_init_thread_info(struct thread_info *ti)
+static inline void klp_init_thread_info(struct task_struct *p)
{
+ struct thread_info *ti = task_thread_info(p);
/* + 1 to account for STACK_END_MAGIC */
- ti->livepatch_sp = (unsigned long *)(ti + 1) + 1;
+ ti->livepatch_sp = end_of_stack(p) + 1;
}
#else
-static void klp_init_thread_info(struct thread_info *ti) { }
+static inline void klp_init_thread_info(struct task_struct *p) { }
#endif /* CONFIG_LIVEPATCH */

#endif /* _ASM_POWERPC_LIVEPATCH_H */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index d9d4eb2ea6c9..a12307ebb7ef 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1632,7 +1632,7 @@ int copy_thread(unsigned long clone_flags,
unsigned long usp,
unsigned long sp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
struct thread_info *ti = task_thread_info(p);

- klp_init_thread_info(ti);
+ klp_init_thread_info(p);

/* Copy registers */
sp -= sizeof(struct pt_regs);
diff --git a/arch/powerpc/kernel/setup-common.c
b/arch/powerpc/kernel/setup-common.c
index 9ca9db707bcb..8054a7b9e026 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -940,7 +940,7 @@ void __init setup_arch(char **cmdline_p)
/* Reserve large chunks of memory for use by CMA for KVM. */
kvm_cma_reserve();

- klp_init_thread_info(&init_thread_info);
+ klp_init_thread_info(&init_task);

init_mm.start_code = (unsigned long)_stext;
init_mm.end_code = (unsigned long) _etext;
Christophe

\
 
 \ /
  Last update: 2018-10-07 18:37    [W:0.084 / U:0.792 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site