| Message ID | 20201015150159.28933-9-cmr@codefail.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Improve signal performance on PPC64 with KUAP | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/merge (118be7377c97e35c33819bcb3bbbae5a42a4ac43) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/next (a2d0230b91f7e23ceb5d8fb6a9799f30517ec33a) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch linus/master (3e4fb4346c781068610d03c12b16c0cfb0fd24a3) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/fixes (0460534b532e5518c657c7d6492b9337d975eaa3) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch linux-next (03d992bd2de6c7f2c9bbd4260ff104c0926ce3ff) |
| snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
Le 15/10/2020 à 17:01, Christopher M. Riedl a écrit : > From: Daniel Axtens <dja@axtens.net> > > Add uaccess blocks and use the 'unsafe' versions of functions doing user > access where possible to reduce the number of times uaccess has to be > opened/closed. > > Signed-off-by: Daniel Axtens <dja@axtens.net> > Signed-off-by: Christopher M. Riedl <cmr@codefail.de> > --- > arch/powerpc/kernel/signal_64.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c > index 3b97e3681a8f..0f4ff7a5bfc1 100644 > --- a/arch/powerpc/kernel/signal_64.c > +++ b/arch/powerpc/kernel/signal_64.c > @@ -779,18 +779,22 @@ SYSCALL_DEFINE0(rt_sigreturn) > */ > regs->msr &= ~MSR_TS_MASK; > > - if (__get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR])) > + if (!user_read_access_begin(uc, sizeof(*uc))) > goto badframe; > + > + unsafe_get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR], badframe_block); > + > if (MSR_TM_ACTIVE(msr)) { > /* We recheckpoint on return. */ > struct ucontext __user *uc_transact; > > /* Trying to start TM on non TM system */ > if (!cpu_has_feature(CPU_FTR_TM)) > - goto badframe; > + goto badframe_block; > + > + unsafe_get_user(uc_transact, &uc->uc_link, badframe_block); > + user_read_access_end(); user_access_end() only in the if branch ? > > - if (__get_user(uc_transact, &uc->uc_link)) > - goto badframe; > if (restore_tm_sigcontexts(current, &uc->uc_mcontext, > &uc_transact->uc_mcontext)) > goto badframe; > @@ -810,12 +814,13 @@ SYSCALL_DEFINE0(rt_sigreturn) > * causing a TM bad thing. > */ > current->thread.regs->msr &= ~MSR_TS_MASK; > + > +#ifndef CONFIG_PPC_TRANSACTIONAL_MEM > if (!user_read_access_begin(uc, sizeof(*uc))) The matching user_read_access_end() is not in the same #ifndef ? That's dirty and hard to follow. Can you re-organise the code to avoid all those nesting ? > - return -EFAULT; > - if (__unsafe_restore_sigcontext(current, NULL, 1, &uc->uc_mcontext)) { > - user_read_access_end(); > goto badframe; > - } > +#endif > + unsafe_restore_sigcontext(current, NULL, 1, &uc->uc_mcontext, > + badframe_block); > user_read_access_end(); > } > > @@ -825,6 +830,8 @@ SYSCALL_DEFINE0(rt_sigreturn) > set_thread_flag(TIF_RESTOREALL); > return 0; > > +badframe_block: > + user_read_access_end(); > badframe: > signal_fault(current, regs, "rt_sigreturn", uc); > > Christophe
On Fri Oct 16, 2020 at 11:07 AM CDT, Christophe Leroy wrote: > > > Le 15/10/2020 à 17:01, Christopher M. Riedl a écrit : > > From: Daniel Axtens <dja@axtens.net> > > > > Add uaccess blocks and use the 'unsafe' versions of functions doing user > > access where possible to reduce the number of times uaccess has to be > > opened/closed. > > > > Signed-off-by: Daniel Axtens <dja@axtens.net> > > Signed-off-by: Christopher M. Riedl <cmr@codefail.de> > > --- > > arch/powerpc/kernel/signal_64.c | 23 +++++++++++++++-------- > > 1 file changed, 15 insertions(+), 8 deletions(-) > > > > diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c > > index 3b97e3681a8f..0f4ff7a5bfc1 100644 > > --- a/arch/powerpc/kernel/signal_64.c > > +++ b/arch/powerpc/kernel/signal_64.c > > @@ -779,18 +779,22 @@ SYSCALL_DEFINE0(rt_sigreturn) > > */ > > regs->msr &= ~MSR_TS_MASK; > > > > - if (__get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR])) > > + if (!user_read_access_begin(uc, sizeof(*uc))) > > goto badframe; > > + > > + unsafe_get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR], badframe_block); > > + > > if (MSR_TM_ACTIVE(msr)) { > > /* We recheckpoint on return. */ > > struct ucontext __user *uc_transact; > > > > /* Trying to start TM on non TM system */ > > if (!cpu_has_feature(CPU_FTR_TM)) > > - goto badframe; > > + goto badframe_block; > > + > > + unsafe_get_user(uc_transact, &uc->uc_link, badframe_block); > > + user_read_access_end(); > > user_access_end() only in the if branch ? > > > > > - if (__get_user(uc_transact, &uc->uc_link)) > > - goto badframe; > > if (restore_tm_sigcontexts(current, &uc->uc_mcontext, > > &uc_transact->uc_mcontext)) > > goto badframe; > > @@ -810,12 +814,13 @@ SYSCALL_DEFINE0(rt_sigreturn) > > * causing a TM bad thing. > > */ > > current->thread.regs->msr &= ~MSR_TS_MASK; > > + > > +#ifndef CONFIG_PPC_TRANSACTIONAL_MEM > > if (!user_read_access_begin(uc, sizeof(*uc))) > > The matching user_read_access_end() is not in the same #ifndef ? That's > dirty and hard to follow. > Can you re-organise the code to avoid all those nesting ? Yes, thanks for pointing this out. I really wanted to avoid changing too much of the logic inside these functions. But I suppose I ended up creating a mess - I will fix this in the next spin. > > > - return -EFAULT; > > - if (__unsafe_restore_sigcontext(current, NULL, 1, &uc->uc_mcontext)) { > > - user_read_access_end(); > > goto badframe; > > - } > > +#endif > > + unsafe_restore_sigcontext(current, NULL, 1, &uc->uc_mcontext, > > + badframe_block); > > user_read_access_end(); > > } > > > > @@ -825,6 +830,8 @@ SYSCALL_DEFINE0(rt_sigreturn) > > set_thread_flag(TIF_RESTOREALL); > > return 0; > > > > +badframe_block: > > + user_read_access_end(); > > badframe: > > signal_fault(current, regs, "rt_sigreturn", uc); > > > > > > Christophe
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c index 3b97e3681a8f..0f4ff7a5bfc1 100644 --- a/arch/powerpc/kernel/signal_64.c +++ b/arch/powerpc/kernel/signal_64.c @@ -779,18 +779,22 @@ SYSCALL_DEFINE0(rt_sigreturn) */ regs->msr &= ~MSR_TS_MASK; - if (__get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR])) + if (!user_read_access_begin(uc, sizeof(*uc))) goto badframe; + + unsafe_get_user(msr, &uc->uc_mcontext.gp_regs[PT_MSR], badframe_block); + if (MSR_TM_ACTIVE(msr)) { /* We recheckpoint on return. */ struct ucontext __user *uc_transact; /* Trying to start TM on non TM system */ if (!cpu_has_feature(CPU_FTR_TM)) - goto badframe; + goto badframe_block; + + unsafe_get_user(uc_transact, &uc->uc_link, badframe_block); + user_read_access_end(); - if (__get_user(uc_transact, &uc->uc_link)) - goto badframe; if (restore_tm_sigcontexts(current, &uc->uc_mcontext, &uc_transact->uc_mcontext)) goto badframe; @@ -810,12 +814,13 @@ SYSCALL_DEFINE0(rt_sigreturn) * causing a TM bad thing. */ current->thread.regs->msr &= ~MSR_TS_MASK; + +#ifndef CONFIG_PPC_TRANSACTIONAL_MEM if (!user_read_access_begin(uc, sizeof(*uc))) - return -EFAULT; - if (__unsafe_restore_sigcontext(current, NULL, 1, &uc->uc_mcontext)) { - user_read_access_end(); goto badframe; - } +#endif + unsafe_restore_sigcontext(current, NULL, 1, &uc->uc_mcontext, + badframe_block); user_read_access_end(); } @@ -825,6 +830,8 @@ SYSCALL_DEFINE0(rt_sigreturn) set_thread_flag(TIF_RESTOREALL); return 0; +badframe_block: + user_read_access_end(); badframe: signal_fault(current, regs, "rt_sigreturn", uc);