From: Michael Schmitz <schmitzmic@gmail.com>
To: Al Viro <viro@zeniv.linux.org.uk>, linux-m68k@lists.linux-m68k.org
Cc: Geert Uytterhoeven <geert@linux-m68k.org>,
Greg Ungerer <gerg@linux-m68k.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] m68k: handle arrivals of multiple signals correctly
Date: Thu, 16 Sep 2021 10:08:50 +1200 [thread overview]
Message-ID: <d62fd0ae-9d25-ec36-107d-d99f15d52e90@gmail.com> (raw)
In-Reply-To: <YP2dBIAPTaVvHiZ6@zeniv-ca.linux.org.uk>
Hi Al,
On 26/07/21 5:19 am, Al Viro wrote:
> When we have several pending signals, have entered with the kernel
> with large exception frame *and* have already built at least one
> sigframe, regs->stkadj is going to be non-zero and regs->format/sr/pc
> are going to be junk - the real values are in shifted exception stack
> frame we'd built when putting together the first sigframe.
>
> If that happens, subsequent sigframes are going to be garbage.
> Not hard to fix - just need to find the "adjusted" frame first
> and look for format/vector/sr/pc in it.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Looks good to me. What's more, it fixes a number of long standing issues
dating back to the 4.10 ages - see discussions at:
https://lore.kernel.org/r/7517d306-21ad-daa1-a2fb-b273211cb588@gmail.com
https://lore.kernel.org/r/ed2ca322-b957-cd52-8d2f-a8edd2785625@linux-m68k.org
- so should be applied to -stable IMO.
Tested by me on 68030 - Finn Thain did some testing on 68040 and might
add his own tag.
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
Reviewed-by: Michael Schmitz <schmitzmic@gmail.com>
> ---
> arch/m68k/kernel/signal.c | 88 ++++++++++++++++++++++-------------------------
> 1 file changed, 42 insertions(+), 46 deletions(-)
>
> diff --git a/arch/m68k/kernel/signal.c b/arch/m68k/kernel/signal.c
> index 8f215e79e70e..cd11eb101eac 100644
> --- a/arch/m68k/kernel/signal.c
> +++ b/arch/m68k/kernel/signal.c
> @@ -447,7 +447,7 @@ static inline void save_fpu_state(struct sigcontext *sc, struct pt_regs *regs)
>
> if (CPU_IS_060 ? sc->sc_fpstate[2] : sc->sc_fpstate[0]) {
> fpu_version = sc->sc_fpstate[0];
> - if (CPU_IS_020_OR_030 &&
> + if (CPU_IS_020_OR_030 && !regs->stkadj &&
> regs->vector >= (VEC_FPBRUC * 4) &&
> regs->vector <= (VEC_FPNAN * 4)) {
> /* Clear pending exception in 68882 idle frame */
> @@ -510,7 +510,7 @@ static inline int rt_save_fpu_state(struct ucontext __user *uc, struct pt_regs *
> if (!(CPU_IS_060 || CPU_IS_COLDFIRE))
> context_size = fpstate[1];
> fpu_version = fpstate[0];
> - if (CPU_IS_020_OR_030 &&
> + if (CPU_IS_020_OR_030 && !regs->stkadj &&
> regs->vector >= (VEC_FPBRUC * 4) &&
> regs->vector <= (VEC_FPNAN * 4)) {
> /* Clear pending exception in 68882 idle frame */
> @@ -832,18 +832,24 @@ asmlinkage int do_rt_sigreturn(struct pt_regs *regs, struct switch_stack *sw)
> return 0;
> }
>
> +static inline struct pt_regs *rte_regs(struct pt_regs *regs)
> +{
> + return (void *)regs + regs->stkadj;
> +}
> +
> static void setup_sigcontext(struct sigcontext *sc, struct pt_regs *regs,
> unsigned long mask)
> {
> + struct pt_regs *tregs = rte_regs(regs);
> sc->sc_mask = mask;
> sc->sc_usp = rdusp();
> sc->sc_d0 = regs->d0;
> sc->sc_d1 = regs->d1;
> sc->sc_a0 = regs->a0;
> sc->sc_a1 = regs->a1;
> - sc->sc_sr = regs->sr;
> - sc->sc_pc = regs->pc;
> - sc->sc_formatvec = regs->format << 12 | regs->vector;
> + sc->sc_sr = tregs->sr;
> + sc->sc_pc = tregs->pc;
> + sc->sc_formatvec = tregs->format << 12 | tregs->vector;
> save_a5_state(sc, regs);
> save_fpu_state(sc, regs);
> }
> @@ -851,6 +857,7 @@ static void setup_sigcontext(struct sigcontext *sc, struct pt_regs *regs,
> static inline int rt_setup_ucontext(struct ucontext __user *uc, struct pt_regs *regs)
> {
> struct switch_stack *sw = (struct switch_stack *)regs - 1;
> + struct pt_regs *tregs = rte_regs(regs);
> greg_t __user *gregs = uc->uc_mcontext.gregs;
> int err = 0;
>
> @@ -871,9 +878,9 @@ static inline int rt_setup_ucontext(struct ucontext __user *uc, struct pt_regs *
> err |= __put_user(sw->a5, &gregs[13]);
> err |= __put_user(sw->a6, &gregs[14]);
> err |= __put_user(rdusp(), &gregs[15]);
> - err |= __put_user(regs->pc, &gregs[16]);
> - err |= __put_user(regs->sr, &gregs[17]);
> - err |= __put_user((regs->format << 12) | regs->vector, &uc->uc_formatvec);
> + err |= __put_user(tregs->pc, &gregs[16]);
> + err |= __put_user(tregs->sr, &gregs[17]);
> + err |= __put_user((tregs->format << 12) | tregs->vector, &uc->uc_formatvec);
> err |= rt_save_fpu_state(uc, regs);
> return err;
> }
> @@ -890,13 +897,14 @@ static int setup_frame(struct ksignal *ksig, sigset_t *set,
> struct pt_regs *regs)
> {
> struct sigframe __user *frame;
> - int fsize = frame_extra_sizes(regs->format);
> + struct pt_regs *tregs = rte_regs(regs);
> + int fsize = frame_extra_sizes(tregs->format);
> struct sigcontext context;
> int err = 0, sig = ksig->sig;
>
> if (fsize < 0) {
> pr_debug("setup_frame: Unknown frame format %#x\n",
> - regs->format);
> + tregs->format);
> return -EFAULT;
> }
>
> @@ -907,7 +915,7 @@ static int setup_frame(struct ksignal *ksig, sigset_t *set,
>
> err |= __put_user(sig, &frame->sig);
>
> - err |= __put_user(regs->vector, &frame->code);
> + err |= __put_user(tregs->vector, &frame->code);
> err |= __put_user(&frame->sc, &frame->psc);
>
> if (_NSIG_WORDS > 1)
> @@ -934,33 +942,27 @@ static int setup_frame(struct ksignal *ksig, sigset_t *set,
> push_cache ((unsigned long) &frame->retcode);
>
> /*
> - * Set up registers for signal handler. All the state we are about
> - * to destroy is successfully copied to sigframe.
> - */
> - wrusp ((unsigned long) frame);
> - regs->pc = (unsigned long) ksig->ka.sa.sa_handler;
> - adjustformat(regs);
> -
> - /*
> * This is subtle; if we build more than one sigframe, all but the
> * first one will see frame format 0 and have fsize == 0, so we won't
> * screw stkadj.
> */
> - if (fsize)
> + if (fsize) {
> regs->stkadj = fsize;
> -
> - /* Prepare to skip over the extra stuff in the exception frame. */
> - if (regs->stkadj) {
> - struct pt_regs *tregs =
> - (struct pt_regs *)((ulong)regs + regs->stkadj);
> + tregs = rte_regs(regs);
> pr_debug("Performing stackadjust=%04lx\n", regs->stkadj);
> - /* This must be copied with decreasing addresses to
> - handle overlaps. */
> tregs->vector = 0;
> tregs->format = 0;
> - tregs->pc = regs->pc;
> tregs->sr = regs->sr;
> }
> +
> + /*
> + * Set up registers for signal handler. All the state we are about
> + * to destroy is successfully copied to sigframe.
> + */
> + wrusp ((unsigned long) frame);
> + tregs->pc = (unsigned long) ksig->ka.sa.sa_handler;
> + adjustformat(regs);
> +
> return 0;
> }
>
> @@ -968,7 +970,8 @@ static int setup_rt_frame(struct ksignal *ksig, sigset_t *set,
> struct pt_regs *regs)
> {
> struct rt_sigframe __user *frame;
> - int fsize = frame_extra_sizes(regs->format);
> + struct pt_regs *tregs = rte_regs(regs);
> + int fsize = frame_extra_sizes(tregs->format);
> int err = 0, sig = ksig->sig;
>
> if (fsize < 0) {
> @@ -1019,33 +1022,26 @@ static int setup_rt_frame(struct ksignal *ksig, sigset_t *set,
> push_cache ((unsigned long) &frame->retcode);
>
> /*
> - * Set up registers for signal handler. All the state we are about
> - * to destroy is successfully copied to sigframe.
> - */
> - wrusp ((unsigned long) frame);
> - regs->pc = (unsigned long) ksig->ka.sa.sa_handler;
> - adjustformat(regs);
> -
> - /*
> * This is subtle; if we build more than one sigframe, all but the
> * first one will see frame format 0 and have fsize == 0, so we won't
> * screw stkadj.
> */
> - if (fsize)
> + if (fsize) {
> regs->stkadj = fsize;
> -
> - /* Prepare to skip over the extra stuff in the exception frame. */
> - if (regs->stkadj) {
> - struct pt_regs *tregs =
> - (struct pt_regs *)((ulong)regs + regs->stkadj);
> + tregs = rte_regs(regs);
> pr_debug("Performing stackadjust=%04lx\n", regs->stkadj);
> - /* This must be copied with decreasing addresses to
> - handle overlaps. */
> tregs->vector = 0;
> tregs->format = 0;
> - tregs->pc = regs->pc;
> tregs->sr = regs->sr;
> }
> +
> + /*
> + * Set up registers for signal handler. All the state we are about
> + * to destroy is successfully copied to sigframe.
> + */
> + wrusp ((unsigned long) frame);
> + tregs->pc = (unsigned long) ksig->ka.sa.sa_handler;
> + adjustformat(regs);
> return 0;
> }
>
next prev parent reply other threads:[~2021-09-15 22:09 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-25 17:18 [RFC][CFT] signal handling fixes Al Viro
2021-07-25 17:19 ` [PATCH 1/3] m68k: handle arrivals of multiple signals correctly Al Viro
2021-09-15 22:08 ` Michael Schmitz [this message]
2021-07-25 17:19 ` [PATCH 2/3] m68k: update ->thread.esp0 before calling syscall_trace() in ret_from_signal Al Viro
2021-09-15 22:19 ` Michael Schmitz
2021-07-25 17:20 ` [PATCH 3/3] m68k: leave stack mangling to asm wrapper of sigreturn() Al Viro
2021-09-15 23:35 ` Michael Schmitz
2021-09-16 0:19 ` Al Viro
2021-09-16 0:53 ` Michael Schmitz
2021-09-16 3:21 ` Al Viro
2021-09-16 5:02 ` Michael Schmitz
2021-09-16 16:14 ` Al Viro
2021-07-27 10:21 ` [RFC][CFT] signal handling fixes Finn Thain
2021-07-27 14:42 ` Al Viro
2021-07-28 1:23 ` Finn Thain
2021-08-11 1:42 ` Finn Thain
2021-09-16 9:03 ` Finn Thain
2021-09-23 14:43 ` Geert Uytterhoeven
2021-09-23 14:45 ` Geert Uytterhoeven
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=d62fd0ae-9d25-ec36-107d-d99f15d52e90@gmail.com \
--to=schmitzmic@gmail.com \
--cc=geert@linux-m68k.org \
--cc=gerg@linux-m68k.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-m68k@lists.linux-m68k.org \
--cc=viro@zeniv.linux.org.uk \
/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.