Messages in this thread Patch in this message | | | Subject | Re: [PATCH v5 3/9] powerpc: Prepare for moving thread_info into task_struct | From | Christophe Leroy <> | Date | Sun, 7 Oct 2018 05:31:35 +0000 |
| |
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
| |