Message ID | b16389f7-6c62-70b7-59b3-87533c0bcc@redhat.com |
---|---|
State | New |
Headers | show |
Series | target/sh4: fix crashes on signal delivery | expand |
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; > } > } > >
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~
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; } }
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(+)