| Message ID | 20190827033010.28090-2-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | facd04a904ff6cdc6ee85d6e85d500f478a1bec4 |
| Headers | show |
| Series | powerpc/64: syscalls in C | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (0e4523c0b4f64eaf7abe59e143e6bdf8f972acff) |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 0 warnings, 1 checks, 30 lines checked |
Le 27/08/2019 à 05:30, Nicholas Piggin a écrit : > Commit 3033f14ab78c3 ("clone: support passing tls argument via C rather > than pt_regs magic") introduced the HAVE_COPY_THREAD_TLS option. Use it > to avoid a subtle assumption about the argument ordering of clone type > syscalls. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > arch/powerpc/Kconfig | 1 + > arch/powerpc/kernel/process.c | 9 +++++---- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index d8dcd8820369..7477a3263225 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -182,6 +182,7 @@ config PPC > select HAVE_STACKPROTECTOR if PPC64 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r13) > select HAVE_STACKPROTECTOR if PPC32 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r2) > select HAVE_CONTEXT_TRACKING if PPC64 > + select HAVE_COPY_THREAD_TLS > select HAVE_DEBUG_KMEMLEAK > select HAVE_DEBUG_STACKOVERFLOW > select HAVE_DYNAMIC_FTRACE > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c > index 8fc4de0d22b4..24621e7e5033 100644 > --- a/arch/powerpc/kernel/process.c > +++ b/arch/powerpc/kernel/process.c > @@ -1600,8 +1600,9 @@ static void setup_ksp_vsid(struct task_struct *p, unsigned long sp) > /* > * Copy architecture-specific thread state > */ > -int copy_thread(unsigned long clone_flags, unsigned long usp, > - unsigned long kthread_arg, struct task_struct *p) > +int copy_thread_tls(unsigned long clone_flags, unsigned long usp, > + unsigned long kthread_arg, struct task_struct *p, > + unsigned long tls) > { > struct pt_regs *childregs, *kregs; > extern void ret_from_fork(void); > @@ -1642,10 +1643,10 @@ int copy_thread(unsigned long clone_flags, unsigned long usp, > if (clone_flags & CLONE_SETTLS) { > #ifdef CONFIG_PPC64 is_32bit_task() exists and always returns 1 on PPC32 so this gross ifdef in the middle of an if/else is pointless, it should be dropped. > if (!is_32bit_task()) > - childregs->gpr[13] = childregs->gpr[6]; > + childregs->gpr[13] = tls; > else > #endif > - childregs->gpr[2] = childregs->gpr[6]; > + childregs->gpr[2] = tls; > } > > f = ret_from_fork; > Christophe
Christophe Leroy's on August 27, 2019 4:07 pm: > > > Le 27/08/2019 à 05:30, Nicholas Piggin a écrit : >> Commit 3033f14ab78c3 ("clone: support passing tls argument via C rather >> than pt_regs magic") introduced the HAVE_COPY_THREAD_TLS option. Use it >> to avoid a subtle assumption about the argument ordering of clone type >> syscalls. >> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> --- >> arch/powerpc/Kconfig | 1 + >> arch/powerpc/kernel/process.c | 9 +++++---- >> 2 files changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index d8dcd8820369..7477a3263225 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -182,6 +182,7 @@ config PPC >> select HAVE_STACKPROTECTOR if PPC64 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r13) >> select HAVE_STACKPROTECTOR if PPC32 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r2) >> select HAVE_CONTEXT_TRACKING if PPC64 >> + select HAVE_COPY_THREAD_TLS >> select HAVE_DEBUG_KMEMLEAK >> select HAVE_DEBUG_STACKOVERFLOW >> select HAVE_DYNAMIC_FTRACE >> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c >> index 8fc4de0d22b4..24621e7e5033 100644 >> --- a/arch/powerpc/kernel/process.c >> +++ b/arch/powerpc/kernel/process.c >> @@ -1600,8 +1600,9 @@ static void setup_ksp_vsid(struct task_struct *p, unsigned long sp) >> /* >> * Copy architecture-specific thread state >> */ >> -int copy_thread(unsigned long clone_flags, unsigned long usp, >> - unsigned long kthread_arg, struct task_struct *p) >> +int copy_thread_tls(unsigned long clone_flags, unsigned long usp, >> + unsigned long kthread_arg, struct task_struct *p, >> + unsigned long tls) >> { >> struct pt_regs *childregs, *kregs; >> extern void ret_from_fork(void); >> @@ -1642,10 +1643,10 @@ int copy_thread(unsigned long clone_flags, unsigned long usp, >> if (clone_flags & CLONE_SETTLS) { >> #ifdef CONFIG_PPC64 > > is_32bit_task() exists and always returns 1 on PPC32 so this gross ifdef > in the middle of an if/else is pointless, it should be dropped. I could do that as another patch in the series. Thanks, Nick
Le 27/08/2019 à 12:13, Nicholas Piggin a écrit : > Christophe Leroy's on August 27, 2019 4:07 pm: >> >> >> Le 27/08/2019 à 05:30, Nicholas Piggin a écrit : >>> Commit 3033f14ab78c3 ("clone: support passing tls argument via C rather >>> than pt_regs magic") introduced the HAVE_COPY_THREAD_TLS option. Use it >>> to avoid a subtle assumption about the argument ordering of clone type >>> syscalls. >>> >>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >>> --- >>> arch/powerpc/Kconfig | 1 + >>> arch/powerpc/kernel/process.c | 9 +++++---- >>> 2 files changed, 6 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >>> index d8dcd8820369..7477a3263225 100644 >>> --- a/arch/powerpc/Kconfig >>> +++ b/arch/powerpc/Kconfig >>> @@ -182,6 +182,7 @@ config PPC >>> select HAVE_STACKPROTECTOR if PPC64 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r13) >>> select HAVE_STACKPROTECTOR if PPC32 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r2) >>> select HAVE_CONTEXT_TRACKING if PPC64 >>> + select HAVE_COPY_THREAD_TLS >>> select HAVE_DEBUG_KMEMLEAK >>> select HAVE_DEBUG_STACKOVERFLOW >>> select HAVE_DYNAMIC_FTRACE >>> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c >>> index 8fc4de0d22b4..24621e7e5033 100644 >>> --- a/arch/powerpc/kernel/process.c >>> +++ b/arch/powerpc/kernel/process.c >>> @@ -1600,8 +1600,9 @@ static void setup_ksp_vsid(struct task_struct *p, unsigned long sp) >>> /* >>> * Copy architecture-specific thread state >>> */ >>> -int copy_thread(unsigned long clone_flags, unsigned long usp, >>> - unsigned long kthread_arg, struct task_struct *p) >>> +int copy_thread_tls(unsigned long clone_flags, unsigned long usp, >>> + unsigned long kthread_arg, struct task_struct *p, >>> + unsigned long tls) >>> { >>> struct pt_regs *childregs, *kregs; >>> extern void ret_from_fork(void); >>> @@ -1642,10 +1643,10 @@ int copy_thread(unsigned long clone_flags, unsigned long usp, >>> if (clone_flags & CLONE_SETTLS) { >>> #ifdef CONFIG_PPC64 >> >> is_32bit_task() exists and always returns 1 on PPC32 so this gross ifdef >> in the middle of an if/else is pointless, it should be dropped. > > I could do that as another patch in the series. Yes, would be good, because if I do an independant patch for that it will conflict with your series. Thanks Christophe
On Tue, 2019-08-27 at 03:30:06 UTC, Nicholas Piggin wrote: > Commit 3033f14ab78c3 ("clone: support passing tls argument via C rather > than pt_regs magic") introduced the HAVE_COPY_THREAD_TLS option. Use it > to avoid a subtle assumption about the argument ordering of clone type > syscalls. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Patches 1 & 2 applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/facd04a904ff6cdc6ee85d6e85d500f478a1bec4 cheers
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index d8dcd8820369..7477a3263225 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -182,6 +182,7 @@ config PPC select HAVE_STACKPROTECTOR if PPC64 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r13) select HAVE_STACKPROTECTOR if PPC32 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r2) select HAVE_CONTEXT_TRACKING if PPC64 + select HAVE_COPY_THREAD_TLS select HAVE_DEBUG_KMEMLEAK select HAVE_DEBUG_STACKOVERFLOW select HAVE_DYNAMIC_FTRACE diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index 8fc4de0d22b4..24621e7e5033 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -1600,8 +1600,9 @@ static void setup_ksp_vsid(struct task_struct *p, unsigned long sp) /* * Copy architecture-specific thread state */ -int copy_thread(unsigned long clone_flags, unsigned long usp, - unsigned long kthread_arg, struct task_struct *p) +int copy_thread_tls(unsigned long clone_flags, unsigned long usp, + unsigned long kthread_arg, struct task_struct *p, + unsigned long tls) { struct pt_regs *childregs, *kregs; extern void ret_from_fork(void); @@ -1642,10 +1643,10 @@ int copy_thread(unsigned long clone_flags, unsigned long usp, if (clone_flags & CLONE_SETTLS) { #ifdef CONFIG_PPC64 if (!is_32bit_task()) - childregs->gpr[13] = childregs->gpr[6]; + childregs->gpr[13] = tls; else #endif - childregs->gpr[2] = childregs->gpr[6]; + childregs->gpr[2] = tls; } f = ret_from_fork;
Commit 3033f14ab78c3 ("clone: support passing tls argument via C rather than pt_regs magic") introduced the HAVE_COPY_THREAD_TLS option. Use it to avoid a subtle assumption about the argument ordering of clone type syscalls. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/Kconfig | 1 + arch/powerpc/kernel/process.c | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-)