[PATCH] target/sh4: fix crashes on signal delivery

[PATCH] target/sh4: fix crashes on signal delivery

[Mikulas Patocka]

sh4 uses gUSA (general UserSpace Atomicity) to provide atomicity on CPUs
that don't have atomic instructions. A gUSA region that adds 1 to an
atomic variable stored in @R2 looks like this:

  4004b6:       03 c7           mova    4004c4 <gusa+0x10>,r0
  4004b8:       f3 61           mov     r15,r1
  4004ba:       09 00           nop
  4004bc:       fa ef           mov     #-6,r15
  4004be:       22 63           mov.l   @r2,r3
  4004c0:       01 73           add     #1,r3
  4004c2:       32 22           mov.l   r3,@r2
  4004c4:       13 6f           mov     r1,r15

R0 contains a pointer to the end of the gUSA region
R1 contains the saved stack pointer
R15 contains negative length of the gUSA region

When this region is interrupted by a signal, the kernel detects if
R15 >= -128U. If yes, the kernel rolls back PC to the beginning of the
region and restores SP by copying R1 to R15.

The problem happens if we are interrupted by a signal at address 4004c4.
R15 still holds the value -6, but the atomic value was already written by
an instruction at address 4004c2. In this situation we can't undo the
gUSA. The function unwind_gusa does nothing, the signal handler attempts
to push a signal frame to the address -6 and crashes.

This patch fixes it, so that if we are interrupted at the last instruction 
in a gUSA region, we copy R1 to R15 to restore the correct stack pointer 
and avoid crashing.

There's another bug: if we are interrupted in a delay slot, we save the
address of the instruction in the delay slot. We must save the address of
the previous instruction.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: qemu-stable@nongnu.org

---
 linux-user/sh4/signal.c |    8 ++++++++
 1 file changed, 8 insertions(+)

Index: qemu/linux-user/sh4/signal.c
===================================================================
--- qemu.orig/linux-user/sh4/signal.c	2023-09-27 19:02:41.000000000 +0200
+++ qemu/linux-user/sh4/signal.c	2023-09-27 19:55:13.000000000 +0200
@@ -104,6 +104,14 @@ static void unwind_gusa(CPUSH4State *reg
 
         /* Reset the SP to the saved version in R1.  */
         regs->gregs[15] = regs->gregs[1];
+    } else if (regs->gregs[15] >= -128u && regs->pc == regs->gregs[0]) {
+        /* If we are on the last instruction of a gUSA region, we must reset
+           the SP, otherwise we would be pushing the signal context to
+           invalid memory.  */
+        regs->gregs[15] = regs->gregs[1];
+    } else if (regs->flags & TB_FLAG_DELAY_SLOT) {
+        /* If we are in a delay slot, push the previous instruction.  */
+        regs->pc -= 2;
     }
 }
 

Re: [PATCH] target/sh4: fix crashes on signal delivery

[Yoshinori Sato]

On Fri, 29 Sep 2023 01:42:08 +0900,
Mikulas Patocka wrote:
> 
> sh4 uses gUSA (general UserSpace Atomicity) to provide atomicity on CPUs
> that don't have atomic instructions. A gUSA region that adds 1 to an
> atomic variable stored in @R2 looks like this:
> 
>   4004b6:       03 c7           mova    4004c4 <gusa+0x10>,r0
>   4004b8:       f3 61           mov     r15,r1
>   4004ba:       09 00           nop
>   4004bc:       fa ef           mov     #-6,r15
>   4004be:       22 63           mov.l   @r2,r3
>   4004c0:       01 73           add     #1,r3
>   4004c2:       32 22           mov.l   r3,@r2
>   4004c4:       13 6f           mov     r1,r15
> 
> R0 contains a pointer to the end of the gUSA region
> R1 contains the saved stack pointer
> R15 contains negative length of the gUSA region
> 
> When this region is interrupted by a signal, the kernel detects if
> R15 >= -128U. If yes, the kernel rolls back PC to the beginning of the
> region and restores SP by copying R1 to R15.
> 
> The problem happens if we are interrupted by a signal at address 4004c4.
> R15 still holds the value -6, but the atomic value was already written by
> an instruction at address 4004c2. In this situation we can't undo the
> gUSA. The function unwind_gusa does nothing, the signal handler attempts
> to push a signal frame to the address -6 and crashes.
> 
> This patch fixes it, so that if we are interrupted at the last instruction 
> in a gUSA region, we copy R1 to R15 to restore the correct stack pointer 
> and avoid crashing.
> 
> There's another bug: if we are interrupted in a delay slot, we save the
> address of the instruction in the delay slot. We must save the address of
> the previous instruction.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Cc: qemu-stable@nongnu.org

Reviewed-by: Yoshinori Sato <ysato@users.sourcefoege.jp>

> ---
>  linux-user/sh4/signal.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> Index: qemu/linux-user/sh4/signal.c
> ===================================================================
> --- qemu.orig/linux-user/sh4/signal.c	2023-09-27 19:02:41.000000000 +0200
> +++ qemu/linux-user/sh4/signal.c	2023-09-27 19:55:13.000000000 +0200
> @@ -104,6 +104,14 @@ static void unwind_gusa(CPUSH4State *reg
>  
>          /* Reset the SP to the saved version in R1.  */
>          regs->gregs[15] = regs->gregs[1];
> +    } else if (regs->gregs[15] >= -128u && regs->pc == regs->gregs[0]) {
> +        /* If we are on the last instruction of a gUSA region, we must reset
> +           the SP, otherwise we would be pushing the signal context to
> +           invalid memory.  */
> +        regs->gregs[15] = regs->gregs[1];
> +    } else if (regs->flags & TB_FLAG_DELAY_SLOT) {
> +        /* If we are in a delay slot, push the previous instruction.  */
> +        regs->pc -= 2;
>      }
>  }
>  
> 

-- 
Yosinori Sato

Re: [PATCH] target/sh4: fix crashes on signal delivery

[Richard Henderson]

On 9/28/23 09:42, Mikulas Patocka wrote:
> sh4 uses gUSA (general UserSpace Atomicity) to provide atomicity on CPUs
> that don't have atomic instructions. A gUSA region that adds 1 to an
> atomic variable stored in @R2 looks like this:
> 
>    4004b6:       03 c7           mova    4004c4 <gusa+0x10>,r0
>    4004b8:       f3 61           mov     r15,r1
>    4004ba:       09 00           nop
>    4004bc:       fa ef           mov     #-6,r15
>    4004be:       22 63           mov.l   @r2,r3
>    4004c0:       01 73           add     #1,r3
>    4004c2:       32 22           mov.l   r3,@r2
>    4004c4:       13 6f           mov     r1,r15
> 
> R0 contains a pointer to the end of the gUSA region
> R1 contains the saved stack pointer
> R15 contains negative length of the gUSA region
> 
> When this region is interrupted by a signal, the kernel detects if
> R15 >= -128U. If yes, the kernel rolls back PC to the beginning of the
> region and restores SP by copying R1 to R15.
> 
> The problem happens if we are interrupted by a signal at address 4004c4.
> R15 still holds the value -6, but the atomic value was already written by
> an instruction at address 4004c2. In this situation we can't undo the
> gUSA. The function unwind_gusa does nothing, the signal handler attempts
> to push a signal frame to the address -6 and crashes.
> 
> This patch fixes it, so that if we are interrupted at the last instruction
> in a gUSA region, we copy R1 to R15 to restore the correct stack pointer
> and avoid crashing.
> 
> There's another bug: if we are interrupted in a delay slot, we save the
> address of the instruction in the delay slot. We must save the address of
> the previous instruction.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> Cc: qemu-stable@nongnu.org
> 
> ---
>   linux-user/sh4/signal.c |    8 ++++++++
>   1 file changed, 8 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Queued to linux-user-next.


r~