From: David Laight <david.laight.linux@gmail•com>
To: "Christophe Leroy (CS GROUP)" <chleroy@kernel•org>
Cc: Michael Ellerman <mpe@ellerman•id.au>,
Nicholas Piggin <npiggin@gmail•com>,
Madhavan Srinivasan <maddy@linux•ibm.com>,
linux-kernel@vger•kernel.org, linuxppc-dev@lists•ozlabs.org
Subject: Re: [PATCH v1 2/8] powerpc/signal64: Untangle setup_tm_sigcontexts() and user_access_begin()
Date: Fri, 22 May 2026 19:55:57 +0100 [thread overview]
Message-ID: <20260522195557.5fd73f80@pumpkin> (raw)
In-Reply-To: <28a7b0b9-3c17-4f76-af80-f17f4869f854@kernel.org>
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
next prev parent reply other threads:[~2026-05-22 18:56 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-22 9:56 [PATCH v1 0/8] powerpc/signal: Convert to scoped user access Christophe Leroy (CS GROUP)
2026-05-22 9:56 ` [PATCH v1 1/8] powerpc/signal32: " Christophe Leroy (CS GROUP)
2026-05-22 9:56 ` [PATCH v1 2/8] powerpc/signal64: Untangle setup_tm_sigcontexts() and user_access_begin() Christophe Leroy (CS GROUP)
2026-05-22 11:12 ` David Laight
2026-05-22 12:06 ` Christophe Leroy (CS GROUP)
2026-05-22 12:44 ` Christophe Leroy (CS GROUP)
2026-05-22 18:55 ` David Laight [this message]
2026-05-22 9:56 ` [PATCH v1 3/8] powerpc/signal64: Convert to scoped user access Christophe Leroy (CS GROUP)
2026-05-22 9:56 ` [PATCH v1 4/8] powerpc/signal64: Access function descriptor with " Christophe Leroy (CS GROUP)
2026-05-22 9:56 ` [PATCH v1 5/8] powerpc/signal: Include the new stack frame inside the user access block Christophe Leroy (CS GROUP)
2026-05-22 9:56 ` [PATCH v1 6/8] signal: Add unsafe_copy_siginfo_to_user() Christophe Leroy (CS GROUP)
2026-05-22 9:56 ` [PATCH v1 7/8] powerpc/uaccess: Add unsafe_clear_user() Christophe Leroy (CS GROUP)
2026-05-22 9:56 ` [PATCH v1 8/8] powerpc/signal: Use unsafe_copy_siginfo_to_user() Christophe Leroy (CS GROUP)
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=20260522195557.5fd73f80@pumpkin \
--to=david.laight.linux@gmail$(echo .)com \
--cc=chleroy@kernel$(echo .)org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=maddy@linux$(echo .)ibm.com \
--cc=mpe@ellerman$(echo .)id.au \
--cc=npiggin@gmail$(echo .)com \
/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