All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: linux-kernel@vger.kernel.org, hch@infradead.org,
	Paul Mackerras <paulus@samba.org>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH RESEND v3 6/6] powerpc/signal: Use unsafe_copy_siginfo_to_user()
Date: Mon, 13 Sep 2021 10:57:37 -0500	[thread overview]
Message-ID: <87h7eopixa.fsf@disp2133> (raw)
In-Reply-To: <2b179deba4fd4ec0868cdc48a0230dfa3aa5a22f.1631537060.git.christophe.leroy@csgroup.eu> (Christophe Leroy's message of "Mon, 13 Sep 2021 17:19:10 +0200")

Christophe Leroy <christophe.leroy@csgroup.eu> writes:

> Use unsafe_copy_siginfo_to_user() in order to do the copy
> within the user access block.
>
> On an mpc 8321 (book3s/32) the improvment is about 5% on a process
> sending a signal to itself.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> v3: Don't leave compat aside, use the new unsafe_copy_siginfo_to_user32()
> ---
>  arch/powerpc/kernel/signal_32.c | 8 +++-----
>  arch/powerpc/kernel/signal_64.c | 5 +----
>  2 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index ff101e2b3bab..3a2db8af2d65 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -710,9 +710,9 @@ static long restore_tm_user_regs(struct pt_regs *regs, struct mcontext __user *s
>  }
>  #endif
>  
> -#ifdef CONFIG_PPC64
> +#ifndef CONFIG_PPC64
>  
> -#define copy_siginfo_to_user	copy_siginfo_to_user32
> +#define unsafe_copy_siginfo_to_user32	unsafe_copy_siginfo_to_user
>  
>  #endif /* CONFIG_PPC64 */

Any particular reason to reverse the sense of this #ifdef?

Otherwise this change looks much cleaner.

Eric


>  
> @@ -779,15 +779,13 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
>  		asm("dcbst %y0; sync; icbi %y0; sync" :: "Z" (mctx->mc_pad[0]));
>  	}
>  	unsafe_put_sigset_t(&frame->uc.uc_sigmask, oldset, failed);
> +	unsafe_copy_siginfo_to_user32(&frame->info, &ksig->info, failed);
>  
>  	/* create a stack frame for the caller of the handler */
>  	unsafe_put_user(regs->gpr[1], newsp, failed);
>  
>  	user_access_end();
>  
> -	if (copy_siginfo_to_user(&frame->info, &ksig->info))
> -		goto badframe;
> -
>  	regs->link = tramp;
>  
>  #ifdef CONFIG_PPC_FPU_REGS
> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> index d80ff83cacb9..56c0c74aa28c 100644
> --- a/arch/powerpc/kernel/signal_64.c
> +++ b/arch/powerpc/kernel/signal_64.c
> @@ -901,15 +901,12 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
>  	}
>  
>  	unsafe_copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set), badframe_block);
> +	unsafe_copy_siginfo_to_user(&frame->info, &ksig->info, badframe_block);
>  	/* Allocate a dummy caller frame for the signal handler. */
>  	unsafe_put_user(regs->gpr[1], newsp, badframe_block);
>  
>  	user_write_access_end();
>  
> -	/* Save the siginfo outside of the unsafe block. */
> -	if (copy_siginfo_to_user(&frame->info, &ksig->info))
> -		goto badframe;
> -
>  	/* Make sure signal handler doesn't get spurious FP exceptions */
>  	tsk->thread.fp_state.fpscr = 0;

WARNING: multiple messages have this Message-ID (diff)
From: ebiederm@xmission.com (Eric W. Biederman)
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	hch@infradead.org, linux-kernel@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH RESEND v3 6/6] powerpc/signal: Use unsafe_copy_siginfo_to_user()
Date: Mon, 13 Sep 2021 10:57:37 -0500	[thread overview]
Message-ID: <87h7eopixa.fsf@disp2133> (raw)
In-Reply-To: <2b179deba4fd4ec0868cdc48a0230dfa3aa5a22f.1631537060.git.christophe.leroy@csgroup.eu> (Christophe Leroy's message of "Mon, 13 Sep 2021 17:19:10 +0200")

Christophe Leroy <christophe.leroy@csgroup.eu> writes:

> Use unsafe_copy_siginfo_to_user() in order to do the copy
> within the user access block.
>
> On an mpc 8321 (book3s/32) the improvment is about 5% on a process
> sending a signal to itself.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> v3: Don't leave compat aside, use the new unsafe_copy_siginfo_to_user32()
> ---
>  arch/powerpc/kernel/signal_32.c | 8 +++-----
>  arch/powerpc/kernel/signal_64.c | 5 +----
>  2 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index ff101e2b3bab..3a2db8af2d65 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -710,9 +710,9 @@ static long restore_tm_user_regs(struct pt_regs *regs, struct mcontext __user *s
>  }
>  #endif
>  
> -#ifdef CONFIG_PPC64
> +#ifndef CONFIG_PPC64
>  
> -#define copy_siginfo_to_user	copy_siginfo_to_user32
> +#define unsafe_copy_siginfo_to_user32	unsafe_copy_siginfo_to_user
>  
>  #endif /* CONFIG_PPC64 */

Any particular reason to reverse the sense of this #ifdef?

Otherwise this change looks much cleaner.

Eric


>  
> @@ -779,15 +779,13 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t *oldset,
>  		asm("dcbst %y0; sync; icbi %y0; sync" :: "Z" (mctx->mc_pad[0]));
>  	}
>  	unsafe_put_sigset_t(&frame->uc.uc_sigmask, oldset, failed);
> +	unsafe_copy_siginfo_to_user32(&frame->info, &ksig->info, failed);
>  
>  	/* create a stack frame for the caller of the handler */
>  	unsafe_put_user(regs->gpr[1], newsp, failed);
>  
>  	user_access_end();
>  
> -	if (copy_siginfo_to_user(&frame->info, &ksig->info))
> -		goto badframe;
> -
>  	regs->link = tramp;
>  
>  #ifdef CONFIG_PPC_FPU_REGS
> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> index d80ff83cacb9..56c0c74aa28c 100644
> --- a/arch/powerpc/kernel/signal_64.c
> +++ b/arch/powerpc/kernel/signal_64.c
> @@ -901,15 +901,12 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
>  	}
>  
>  	unsafe_copy_to_user(&frame->uc.uc_sigmask, set, sizeof(*set), badframe_block);
> +	unsafe_copy_siginfo_to_user(&frame->info, &ksig->info, badframe_block);
>  	/* Allocate a dummy caller frame for the signal handler. */
>  	unsafe_put_user(regs->gpr[1], newsp, badframe_block);
>  
>  	user_write_access_end();
>  
> -	/* Save the siginfo outside of the unsafe block. */
> -	if (copy_siginfo_to_user(&frame->info, &ksig->info))
> -		goto badframe;
> -
>  	/* Make sure signal handler doesn't get spurious FP exceptions */
>  	tsk->thread.fp_state.fpscr = 0;

  reply	other threads:[~2021-09-13 15:58 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-13 15:19 [PATCH RESEND v3 1/6] powerpc/signal64: Access function descriptor with user access block Christophe Leroy
2021-09-13 15:19 ` Christophe Leroy
2021-09-13 15:19 ` [PATCH RESEND v3 2/6] powerpc/signal: Include the new stack frame inside the " Christophe Leroy
2021-09-13 15:19   ` Christophe Leroy
2021-09-13 15:19 ` [PATCH RESEND v3 3/6] signal: Add unsafe_copy_siginfo_to_user() Christophe Leroy
2021-09-13 15:19   ` Christophe Leroy
2021-09-13 15:19 ` [PATCH RESEND v3 4/6] signal: Add unsafe_copy_siginfo_to_user32() Christophe Leroy
2021-09-13 15:19   ` Christophe Leroy
2021-09-13 15:54   ` Eric W. Biederman
2021-09-13 15:54     ` Eric W. Biederman
2021-09-13 17:01     ` Christophe Leroy
2021-09-13 17:01       ` Christophe Leroy
2021-09-13 15:19 ` [PATCH RESEND v3 5/6] powerpc/uaccess: Add unsafe_clear_user() Christophe Leroy
2021-09-13 15:19   ` Christophe Leroy
2021-09-13 15:19 ` [PATCH RESEND v3 6/6] powerpc/signal: Use unsafe_copy_siginfo_to_user() Christophe Leroy
2021-09-13 15:19   ` Christophe Leroy
2021-09-13 15:57   ` Eric W. Biederman [this message]
2021-09-13 15:57     ` Eric W. Biederman
2021-09-13 16:21     ` Eric W. Biederman
2021-09-13 16:21       ` Eric W. Biederman
2021-09-13 17:19       ` Christophe Leroy
2021-09-13 17:19         ` Christophe Leroy
2021-09-13 19:11         ` Eric W. Biederman
2021-09-13 19:11           ` Eric W. Biederman
2021-09-14 14:00           ` Christophe Leroy
2021-09-14 14:00             ` Christophe Leroy
2021-09-13 17:14     ` Christophe Leroy
2021-09-13 17:14       ` Christophe Leroy

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=87h7eopixa.fsf@disp2133 \
    --to=ebiederm@xmission.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.