From: "Christopher M. Riedl" <cmr@codefail•de>
To: "Christophe Leroy" <christophe.leroy@csgroup•eu>,
<linuxppc-dev@lists•ozlabs.org>
Cc: Daniel Axtens <dja@axtens•net>
Subject: Re: [PATCH 7/8] powerpc/signal64: Rewrite handle_rt_signal64() to minimise uaccess switches
Date: Mon, 19 Oct 2020 21:44:26 -0500 [thread overview]
Message-ID: <C6HDGA21FU6I.17O37JPOYTU74@geist> (raw)
In-Reply-To: <1cedcf43-4594-5db1-d248-7c06a572aecc@csgroup.eu>
On Fri Oct 16, 2020 at 11:00 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.
> >
> > There is no 'unsafe' version of copy_siginfo_to_user, so move it
> > slightly to allow for a "longer" uaccess block.
> >
> > Signed-off-by: Daniel Axtens <dja@axtens•net>
> > Signed-off-by: Christopher M. Riedl <cmr@codefail•de>
> > ---
> > arch/powerpc/kernel/signal_64.c | 54 ++++++++++++++++-----------------
> > 1 file changed, 27 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> > index 6d4f7a5c4fbf..3b97e3681a8f 100644
> > --- a/arch/powerpc/kernel/signal_64.c
> > +++ b/arch/powerpc/kernel/signal_64.c
> > @@ -843,46 +843,42 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
> > /* Save the thread's msr before get_tm_stackpointer() changes it */
> > unsigned long msr = regs->msr;
> > #endif
> > -
> > frame = get_sigframe(ksig, tsk, sizeof(*frame), 0);
> > - if (!access_ok(frame, sizeof(*frame)))
> > + if (!user_write_access_begin(frame, sizeof(*frame)))
> > goto badframe;
> >
> > - err |= __put_user(&frame->info, &frame->pinfo);
> > - err |= __put_user(&frame->uc, &frame->puc);
> > - err |= copy_siginfo_to_user(&frame->info, &ksig->info);
> > - if (err)
> > - goto badframe;
> > + unsafe_put_user(&frame->info, &frame->pinfo, badframe_block);
> > + unsafe_put_user(&frame->uc, &frame->puc, badframe_block);
> >
> > /* Create the ucontext. */
> > - err |= __put_user(0, &frame->uc.uc_flags);
> > - err |= __save_altstack(&frame->uc.uc_stack, regs->gpr[1]);
> > + unsafe_put_user(0, &frame->uc.uc_flags, badframe_block);
> > + unsafe_save_altstack(&frame->uc.uc_stack, regs->gpr[1], badframe_block);
> > +
> > #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> > if (MSR_TM_ACTIVE(msr)) {
> > /* The ucontext_t passed to userland points to the second
> > * ucontext_t (for transactional state) with its uc_link ptr.
> > */
> > - err |= __put_user(&frame->uc_transact, &frame->uc.uc_link);
> > + unsafe_put_user(&frame->uc_transact, &frame->uc.uc_link, badframe_block);
> > + user_write_access_end();
>
> Whaou. Doing this inside an #ifdef sequence is dirty.
> Can you reorganise code to avoid that and to avoid nesting #ifdef/#endif
> and the if/else as I did in
> signal32 ?
Hopefully yes - next spin!
>
> > err |= setup_tm_sigcontexts(&frame->uc.uc_mcontext,
> > &frame->uc_transact.uc_mcontext,
> > tsk, ksig->sig, NULL,
> > (unsigned long)ksig->ka.sa.sa_handler,
> > msr);
> > + if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
> > + goto badframe;
> > +
> > } else
> > #endif
> > {
> > - err |= __put_user(0, &frame->uc.uc_link);
> > -
> > - if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
> > - return -EFAULT;
> > - err |= __unsafe_setup_sigcontext(&frame->uc.uc_mcontext, tsk,
> > - ksig->sig, NULL,
> > - (unsigned long)ksig->ka.sa.sa_handler, 1);
> > - user_write_access_end();
> > + unsafe_put_user(0, &frame->uc.uc_link, badframe_block);
> > + unsafe_setup_sigcontext(&frame->uc.uc_mcontext, tsk, ksig->sig,
> > + NULL, (unsigned long)ksig->ka.sa.sa_handler,
> > + 1, badframe_block);
> > }
> > - err |= __copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set));
> > - if (err)
> > - goto badframe;
> > +
> > + unsafe_copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set), badframe_block);
> >
> > /* Make sure signal handler doesn't get spurious FP exceptions */
> > tsk->thread.fp_state.fpscr = 0;
> > @@ -891,15 +887,17 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
> > if (vdso64_rt_sigtramp && tsk->mm->context.vdso_base) {
> > regs->nip = tsk->mm->context.vdso_base + vdso64_rt_sigtramp;
> > } else {
> > - if (!user_write_access_begin(frame, sizeof(struct rt_sigframe)))
> > - return -EFAULT;
> > - err |= __unsafe_setup_trampoline(__NR_rt_sigreturn, &frame->tramp[0]);
> > - user_write_access_end();
> > - if (err)
> > - goto badframe;
> > + unsafe_setup_trampoline(__NR_rt_sigreturn, &frame->tramp[0],
> > + badframe_block);
> > regs->nip = (unsigned long) &frame->tramp[0];
> > }
> >
> > + user_write_access_end();
> > +
> > + /* Save the siginfo outside of the unsafe block. */
> > + if (copy_siginfo_to_user(&frame->info, &ksig->info))
> > + goto badframe;
> > +
> > /* Allocate a dummy caller frame for the signal handler. */
> > newsp = ((unsigned long)frame) - __SIGNAL_FRAMESIZE;
> > err |= put_user(regs->gpr[1], (unsigned long __user *)newsp);
> > @@ -939,6 +937,8 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
> >
> > return 0;
> >
> > +badframe_block:
> > + user_write_access_end();
> > badframe:
> > signal_fault(current, regs, "handle_rt_signal64", frame);
> >
> >
>
>
> Christophe
next prev parent reply other threads:[~2020-10-20 2:44 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-15 15:01 [PATCH 0/8] Improve signal performance on PPC64 with KUAP Christopher M. Riedl
2020-10-15 15:01 ` [PATCH 1/8] powerpc/uaccess: Add unsafe_copy_from_user Christopher M. Riedl
2020-10-16 6:54 ` Christoph Hellwig
2020-10-16 13:18 ` Christophe Leroy
2020-10-16 13:17 ` Christophe Leroy
2020-10-20 3:00 ` Christopher M. Riedl
2020-10-15 15:01 ` [PATCH 2/8] powerpc/signal: Add unsafe_copy_{vsx,fpr}_from_user() Christopher M. Riedl
2020-10-16 13:48 ` [PATCH 2/8] powerpc/signal: Add unsafe_copy_{vsx, fpr}_from_user() Christophe Leroy
2020-10-20 2:01 ` [PATCH 2/8] powerpc/signal: Add unsafe_copy_{vsx,fpr}_from_user() Christopher M. Riedl
2021-02-06 16:32 ` [PATCH 2/8] powerpc/signal: Add unsafe_copy_{vsx, fpr}_from_user() Christophe Leroy
2021-02-06 17:39 ` [PATCH 2/8] powerpc/signal: Add unsafe_copy_{vsx,fpr}_from_user() Christopher M. Riedl
2021-02-07 10:12 ` [PATCH 2/8] powerpc/signal: Add unsafe_copy_{vsx, fpr}_from_user() Christophe Leroy
2021-02-08 17:14 ` [PATCH 2/8] powerpc/signal: Add unsafe_copy_{vsx,fpr}_from_user() Christopher M. Riedl
2021-02-08 17:18 ` [PATCH 2/8] powerpc/signal: Add unsafe_copy_{vsx, fpr}_from_user() Christophe Leroy
2020-10-15 15:01 ` [PATCH 3/8] powerpc: Mark functions called inside uaccess blocks w/ 'notrace' Christopher M. Riedl
2020-10-16 6:56 ` Christoph Hellwig
2020-10-16 9:41 ` Peter Zijlstra
2020-10-20 7:34 ` Michael Ellerman
2020-10-16 7:02 ` Christophe Leroy
2020-10-20 1:59 ` Christopher M. Riedl
2020-10-15 15:01 ` [PATCH 4/8] powerpc/signal64: Replace setup_sigcontext() w/ unsafe_setup_sigcontext() Christopher M. Riedl
2020-10-15 15:01 ` [PATCH 5/8] powerpc/signal64: Replace restore_sigcontext() w/ unsafe_restore_sigcontext() Christopher M. Riedl
2020-10-15 15:01 ` [PATCH 6/8] powerpc/signal64: Replace setup_trampoline() w/ unsafe_setup_trampoline() Christopher M. Riedl
2020-10-16 13:56 ` Christophe Leroy
2020-10-20 2:42 ` Christopher M. Riedl
2020-10-20 5:02 ` Christophe Leroy
2020-10-15 15:01 ` [PATCH 7/8] powerpc/signal64: Rewrite handle_rt_signal64() to minimise uaccess switches Christopher M. Riedl
2020-10-16 14:00 ` Christophe Leroy
2020-10-20 2:44 ` Christopher M. Riedl [this message]
2020-10-15 15:01 ` [PATCH 8/8] powerpc/signal64: Rewrite rt_sigreturn() " Christopher M. Riedl
2020-10-16 14:07 ` Christophe Leroy
2020-10-20 2:45 ` Christopher M. Riedl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=C6HDGA21FU6I.17O37JPOYTU74@geist \
--to=cmr@codefail$(echo .)de \
--cc=christophe.leroy@csgroup$(echo .)eu \
--cc=dja@axtens$(echo .)net \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox