Re: [PATCH v1 2/8] powerpc/signal64: Untangle setup_tm_sigcontexts() and user_access_begin()

[David Laight <david.laight.linux@gmail.com>]

On Fri, 22 May 2026 14:44:55 +0200
"Christophe Leroy (CS GROUP)" <chleroy@kernel.org> wrote:

> Le 22/05/2026 à 14:06, Christophe Leroy (CS GROUP) a écrit :
> > 
> > 
> > Le 22/05/2026 à 13:12, David Laight a écrit :  
> >> On Fri, 22 May 2026 11:56:02 +0200
> >> "Christophe Leroy (CS GROUP)" <chleroy@kernel.org> wrote:
> >>  
> >>> Call setup_tm_sigcontexts() before opening user access to avoid
> >>> having to close and open again.
> >>>
> >>> Signed-off-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>
> >>> ---
> >>>   arch/powerpc/kernel/signal_64.c | 22 +++++++++-------------
> >>>   1 file changed, 9 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/ 
> >>> signal_64.c
> >>> index 86bb5bb4c143..3849af21e1d8 100644
> >>> --- a/arch/powerpc/kernel/signal_64.c
> >>> +++ b/arch/powerpc/kernel/signal_64.c
> >>> @@ -873,6 +873,15 @@ int handle_rt_signal64(struct ksignal *ksig, 
> >>> sigset_t *set,
> >>>       if (!MSR_TM_ACTIVE(msr))
> >>>           prepare_setup_sigcontext(tsk);
> >>> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> >>> +    if (MSR_TM_ACTIVE(msr))  
> >>
> >> Can't that be done without the ugly #ifdef?
> >> I assume MSR_TM_ACTIVE() will be zero - so it will all get optimised 
> >> away.  
> > 
> > Yes but struct rt_sigframe field uc_transact only exists when 
> > CONFIG_PPC_TRANSACTIONAL_MEM is defined.
> > 
> > And that would also require a stub setup_tm_sigcontexts()  
> 
> After thinking once more, I think we can do the following, is it better 
> for you ?

Certainly more like the expected style.
setup_tm_sigcontexts() will get inlined, so it doesn't matter what values
are passed as the arguments.

-- David

> 
> diff --git a/arch/powerpc/kernel/signal_64.c 
> b/arch/powerpc/kernel/signal_64.c
> index 86bb5bb4c143..c70732e8002d 100644
> --- a/arch/powerpc/kernel/signal_64.c
> +++ b/arch/powerpc/kernel/signal_64.c
> @@ -203,8 +203,7 @@ static long notrace __unsafe_setup_sigcontext(struct 
> sigcontext __user *sc,
>    * examine the transactional registers in the 2nd sigcontext to 
> determine the
>    * real origin of the signal.
>    */
> -static long setup_tm_sigcontexts(struct sigcontext __user *sc,
> -				 struct sigcontext __user *tm_sc,
> +static long setup_tm_sigcontexts(struct rt_sigframe __user *frame,
>   				 struct task_struct *tsk,
>   				 int signr, sigset_t *set, unsigned long handler,
>   				 unsigned long msr)
> @@ -217,6 +216,8 @@ static long setup_tm_sigcontexts(struct sigcontext 
> __user *sc,
>   	 * Userland shall check AT_HWCAP to know wether it can rely on the
>   	 * v_regs pointer or not.
>   	 */
> +	struct sigcontext __user *sc = &frame->uc.uc_mcontext;
> +	struct sigcontext __user *tm_sc = &frame->uc_transact.uc_mcontext;
>   #ifdef CONFIG_ALTIVEC
>   	elf_vrreg_t __user *v_regs = sigcontext_vmx_regs(sc);
>   	elf_vrreg_t __user *tm_v_regs = sigcontext_vmx_regs(tm_sc);
> @@ -325,6 +326,14 @@ static long setup_tm_sigcontexts(struct sigcontext 
> __user *sc,
> 
>   	return err;
>   }
> +#else
> +static long setup_tm_sigcontexts(struct rt_sigframe __user *frame,
> +				 struct task_struct *tsk,
> +				 int signr, sigset_t *set, unsigned long handler,
> +				 unsigned long msr)
> +{
> +	return -EINVAL;
> +}
>   #endif
> 
>   /*
> @@ -872,6 +881,9 @@ int handle_rt_signal64(struct ksignal *ksig, 
> sigset_t *set,
>   	 */
>   	if (!MSR_TM_ACTIVE(msr))
>   		prepare_setup_sigcontext(tsk);
> +	else
> +		err |= setup_tm_sigcontexts(frame, tsk, ksig->sig, NULL,
> +					    (unsigned long)ksig->ka.sa.sa_handler, msr);
> 
>   	if (!user_write_access_begin(frame, sizeof(*frame)))
>   		goto badframe;
> @@ -889,19 +901,6 @@ int handle_rt_signal64(struct ksignal *ksig, 
> sigset_t *set,
>   		 * ucontext_t (for transactional state) with its uc_link ptr.
>   		 */
>   		unsafe_put_user(&frame->uc_transact, &frame->uc.uc_link, 
> badframe_block);
> -
> -		user_write_access_end();
> -
> -		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->uc.uc_sigmask,
> -					     sizeof(frame->uc.uc_sigmask)))
> -			goto badframe;
> -
>   #endif
>   	} else {
>   		unsafe_put_user(0, &frame->uc.uc_link, badframe_block);
> 
> 
> Christophe