Re: [PATCH v1 2/8] powerpc/signal64: Untangle setup_tm_sigcontexts() and user_access_begin()
From: David Laight <hidden>
Date: 2026-05-22 18:56:01
Also in:
lkml
On Fri, 22 May 2026 14:44:55 +0200 "Christophe Leroy (CS GROUP)" [off-list ref] wrote:
Le 22/05/2026 à 14:06, Christophe Leroy (CS GROUP) a écrit :quoted
Le 22/05/2026 à 13:12, David Laight a écrit :quoted
On Fri, 22 May 2026 11:56:02 +0200 "Christophe Leroy (CS GROUP)" [off-list ref] wrote:quoted
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
quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/kernel/signal_64.cb/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(structsigcontext __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