* [PATCH 0/4] powerpc: clean up pt_regs traps handling
@ 2020-04-21 2:19 Nicholas Piggin
2020-04-21 2:19 ` [PATCH 1/4] powerpc/64s: Always has full regs, so remove remnant checks Nicholas Piggin
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Nicholas Piggin @ 2020-04-21 2:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
scv support needs to test trap in some cases to distinguish sc from scv,
so it helps to have a few tidy-up patches to start with. This turned
into a slightly bigger job that we needed to do to clean up the double
restart logic that today zeroes traps which is ugly.
Thanks,
Nick
Nicholas Piggin (4):
powerpc/64s: Always has full regs, so remove remnant checks
powerpc: Use SET_TRAP and avoid open-coding trap masking
powerpc: TRAP_IS_SYSCALL helper to hide syscall trap number
powerpc: Use trap metadata to prevent double restart rather than
zeroing trap
arch/powerpc/include/asm/ptrace.h | 31 +++-
arch/powerpc/include/asm/syscall.h | 5 +-
arch/powerpc/kernel/process.c | 4 +-
arch/powerpc/kernel/ptrace/ptrace-tm.c | 2 +-
arch/powerpc/kernel/ptrace/ptrace-view.c | 2 +-
arch/powerpc/kernel/signal.c | 9 +-
arch/powerpc/kernel/signal_32.c | 2 +-
arch/powerpc/kernel/signal_64.c | 10 +-
arch/powerpc/xmon/xmon.c | 4 +-
.../testing/selftests/powerpc/signal/Makefile | 2 +-
.../powerpc/signal/sig_sc_double_restart.c | 174 ++++++++++++++++++
11 files changed, 220 insertions(+), 25 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/signal/sig_sc_double_restart.c
--
2.23.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/4] powerpc/64s: Always has full regs, so remove remnant checks
2020-04-21 2:19 [PATCH 0/4] powerpc: clean up pt_regs traps handling Nicholas Piggin
@ 2020-04-21 2:19 ` Nicholas Piggin
2020-04-21 2:19 ` [PATCH 2/4] powerpc: Use SET_TRAP and avoid open-coding trap masking Nicholas Piggin
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Nicholas Piggin @ 2020-04-21 2:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/ptrace.h | 23 ++++++++++++++++-------
arch/powerpc/kernel/process.c | 2 +-
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index e0195e6b892b..89f31d5a8062 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -179,6 +179,20 @@ extern int ptrace_put_reg(struct task_struct *task, int regno,
#define current_pt_regs() \
((struct pt_regs *)((unsigned long)task_stack_page(current) + THREAD_SIZE) - 1)
+
+#ifdef __powerpc64__
+#ifdef CONFIG_PPC_BOOK3S
+#define TRAP(regs) ((regs)->trap)
+#define FULL_REGS(regs) true
+#define SET_FULL_REGS(regs) do { } while (0)
+#else
+#define TRAP(regs) ((regs)->trap & ~0x1)
+#define FULL_REGS(regs) (((regs)->trap & 1) == 0)
+#define SET_FULL_REGS(regs) ((regs)->trap |= 1)
+#endif
+#define CHECK_FULL_REGS(regs) BUG_ON(!FULL_REGS(regs))
+#define NV_REG_POISON 0xdeadbeefdeadbeefUL
+#else
/*
* We use the least-significant bit of the trap field to indicate
* whether we have saved the full set of registers, or only a
@@ -186,17 +200,12 @@ extern int ptrace_put_reg(struct task_struct *task, int regno,
* On 4xx we use the next bit to indicate whether the exception
* is a critical exception (1 means it is).
*/
+#define TRAP(regs) ((regs)->trap & ~0xF)
#define FULL_REGS(regs) (((regs)->trap & 1) == 0)
-#ifndef __powerpc64__
+#define SET_FULL_REGS(regs) ((regs)->trap |= 1)
#define IS_CRITICAL_EXC(regs) (((regs)->trap & 2) != 0)
#define IS_MCHECK_EXC(regs) (((regs)->trap & 4) != 0)
#define IS_DEBUG_EXC(regs) (((regs)->trap & 8) != 0)
-#endif /* ! __powerpc64__ */
-#define TRAP(regs) ((regs)->trap & ~0xF)
-#ifdef __powerpc64__
-#define NV_REG_POISON 0xdeadbeefdeadbeefUL
-#define CHECK_FULL_REGS(regs) BUG_ON(regs->trap & 1)
-#else
#define NV_REG_POISON 0xdeadbeef
#define CHECK_FULL_REGS(regs) \
do { \
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 9c21288f8645..6599a3099d10 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1740,7 +1740,7 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)
* FULL_REGS(regs) return true. This is necessary to allow
* ptrace to examine the thread immediately after exec.
*/
- regs->trap &= ~1UL;
+ SET_FULL_REGS(regs);
#ifdef CONFIG_PPC32
regs->mq = 0;
--
2.23.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/4] powerpc: Use SET_TRAP and avoid open-coding trap masking
2020-04-21 2:19 [PATCH 0/4] powerpc: clean up pt_regs traps handling Nicholas Piggin
2020-04-21 2:19 ` [PATCH 1/4] powerpc/64s: Always has full regs, so remove remnant checks Nicholas Piggin
@ 2020-04-21 2:19 ` Nicholas Piggin
2020-04-21 2:19 ` [PATCH 3/4] powerpc: TRAP_IS_SYSCALL helper to hide syscall trap number Nicholas Piggin
2020-04-21 2:19 ` [PATCH 4/4] powerpc: Use trap metadata to prevent double restart rather than zeroing trap Nicholas Piggin
3 siblings, 0 replies; 5+ messages in thread
From: Nicholas Piggin @ 2020-04-21 2:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
The pt_regs.trap field keeps 4 low bits for some metadata about the
trap or how it was handled, which is masked off in order to test the
architectural trap number.
Add a SET_TRAP() accessor to set this, equivalent to TRAP() for
returning it. This is actually not quite the equivalent of TRAP()
because it always clears the low bits, which may be harmless if
it can only be updated via ptrace syscall, but it seems dangerous.
In fact settting TRAP from ptrace doesn't seem like a great idea
so maybe it's better deleted.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/ptrace.h | 3 +++
arch/powerpc/kernel/ptrace/ptrace-tm.c | 2 +-
arch/powerpc/kernel/ptrace/ptrace-view.c | 2 +-
arch/powerpc/xmon/xmon.c | 2 +-
4 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 89f31d5a8062..7eaa2ecfd0b7 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -183,10 +183,12 @@ extern int ptrace_put_reg(struct task_struct *task, int regno,
#ifdef __powerpc64__
#ifdef CONFIG_PPC_BOOK3S
#define TRAP(regs) ((regs)->trap)
+#define SET_TRAP(regs, val) ((regs)->trap = (val))
#define FULL_REGS(regs) true
#define SET_FULL_REGS(regs) do { } while (0)
#else
#define TRAP(regs) ((regs)->trap & ~0x1)
+#define SET_TRAP(regs, val) ((regs)->trap = ((regs)->trap & 0x1) | ((val) & ~0x1))
#define FULL_REGS(regs) (((regs)->trap & 1) == 0)
#define SET_FULL_REGS(regs) ((regs)->trap |= 1)
#endif
@@ -201,6 +203,7 @@ extern int ptrace_put_reg(struct task_struct *task, int regno,
* is a critical exception (1 means it is).
*/
#define TRAP(regs) ((regs)->trap & ~0xF)
+#define SET_TRAP(regs, val) ((regs)->trap = ((regs)->trap & 0xF) | ((val) & ~0xF))
#define FULL_REGS(regs) (((regs)->trap & 1) == 0)
#define SET_FULL_REGS(regs) ((regs)->trap |= 1)
#define IS_CRITICAL_EXC(regs) (((regs)->trap & 2) != 0)
diff --git a/arch/powerpc/kernel/ptrace/ptrace-tm.c b/arch/powerpc/kernel/ptrace/ptrace-tm.c
index d75aff31f637..ac76b2924a1a 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-tm.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-tm.c
@@ -43,7 +43,7 @@ static int set_user_ckpt_msr(struct task_struct *task, unsigned long msr)
static int set_user_ckpt_trap(struct task_struct *task, unsigned long trap)
{
- task->thread.ckpt_regs.trap = trap & 0xfff0;
+ SET_TRAP(&task->thread.ckpt_regs, trap);
return 0;
}
diff --git a/arch/powerpc/kernel/ptrace/ptrace-view.c b/arch/powerpc/kernel/ptrace/ptrace-view.c
index 15e3b79b6395..87c8bd67512c 100644
--- a/arch/powerpc/kernel/ptrace/ptrace-view.c
+++ b/arch/powerpc/kernel/ptrace/ptrace-view.c
@@ -149,7 +149,7 @@ static int set_user_dscr(struct task_struct *task, unsigned long dscr)
*/
static int set_user_trap(struct task_struct *task, unsigned long trap)
{
- task->thread.regs->trap = trap & 0xfff0;
+ SET_TRAP(task->thread.regs, trap);
return 0;
}
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 7af840c0fc93..20c1fc08819b 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1178,7 +1178,7 @@ static int do_step(struct pt_regs *regs)
return 0;
}
if (stepped > 0) {
- regs->trap = 0xd00 | (regs->trap & 1);
+ SET_TRAP(regs, 0xd00);
printf("stepped to ");
xmon_print_symbol(regs->nip, " ", "\n");
ppc_inst_dump(regs->nip, 1, 0);
--
2.23.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/4] powerpc: TRAP_IS_SYSCALL helper to hide syscall trap number
2020-04-21 2:19 [PATCH 0/4] powerpc: clean up pt_regs traps handling Nicholas Piggin
2020-04-21 2:19 ` [PATCH 1/4] powerpc/64s: Always has full regs, so remove remnant checks Nicholas Piggin
2020-04-21 2:19 ` [PATCH 2/4] powerpc: Use SET_TRAP and avoid open-coding trap masking Nicholas Piggin
@ 2020-04-21 2:19 ` Nicholas Piggin
2020-04-21 2:19 ` [PATCH 4/4] powerpc: Use trap metadata to prevent double restart rather than zeroing trap Nicholas Piggin
3 siblings, 0 replies; 5+ messages in thread
From: Nicholas Piggin @ 2020-04-21 2:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
A new system call interrupt will be added with a new trap number.
Hide the explicit 0xc00 test behind an accessor to reduce churn
in callers.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/ptrace.h | 2 ++
arch/powerpc/include/asm/syscall.h | 5 ++++-
arch/powerpc/kernel/process.c | 2 +-
arch/powerpc/kernel/signal.c | 2 +-
arch/powerpc/xmon/xmon.c | 2 +-
5 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 7eaa2ecfd0b7..5eb249c725bd 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -180,6 +180,8 @@ extern int ptrace_put_reg(struct task_struct *task, int regno,
#define current_pt_regs() \
((struct pt_regs *)((unsigned long)task_stack_page(current) + THREAD_SIZE) - 1)
+#define TRAP_IS_SYSCALL(regs) (TRAP(regs) == 0xc00)
+
#ifdef __powerpc64__
#ifdef CONFIG_PPC_BOOK3S
#define TRAP(regs) ((regs)->trap)
diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
index 38d62acfdce7..1e0446d6ba45 100644
--- a/arch/powerpc/include/asm/syscall.h
+++ b/arch/powerpc/include/asm/syscall.h
@@ -26,7 +26,10 @@ static inline int syscall_get_nr(struct task_struct *task, struct pt_regs *regs)
* This is important for seccomp so that compat tasks can set r0 = -1
* to reject the syscall.
*/
- return TRAP(regs) == 0xc00 ? regs->gpr[0] : -1;
+ if (TRAP_IS_SYSCALL(regs))
+ return regs->gpr[0];
+ else
+ return -1;
}
static inline void syscall_rollback(struct task_struct *task,
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 6599a3099d10..0dce642ca39d 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1412,7 +1412,7 @@ void show_regs(struct pt_regs * regs)
print_msr_bits(regs->msr);
pr_cont(" CR: %08lx XER: %08lx\n", regs->ccr, regs->xer);
trap = TRAP(regs);
- if ((TRAP(regs) != 0xc00) && cpu_has_feature(CPU_FTR_CFAR))
+ if (!TRAP_IS_SYSCALL(regs) && cpu_has_feature(CPU_FTR_CFAR))
pr_cont("CFAR: "REG" ", regs->orig_gpr3);
if (trap == 0x200 || trap == 0x300 || trap == 0x600)
#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index a264989626fd..4b74eef1d881 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -198,7 +198,7 @@ static void check_syscall_restart(struct pt_regs *regs, struct k_sigaction *ka,
int restart = 1;
/* syscall ? */
- if (TRAP(regs) != 0x0C00)
+ if (!TRAP_IS_SYSCALL(regs))
return;
/* error signalled ? */
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 20c1fc08819b..8da771e025fa 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -1776,7 +1776,7 @@ static void prregs(struct pt_regs *fp)
#endif
printf("pc = ");
xmon_print_symbol(fp->nip, " ", "\n");
- if (TRAP(fp) != 0xc00 && cpu_has_feature(CPU_FTR_CFAR)) {
+ if (!TRAP_IS_SYSCALL(fp) && cpu_has_feature(CPU_FTR_CFAR)) {
printf("cfar= ");
xmon_print_symbol(fp->orig_gpr3, " ", "\n");
}
--
2.23.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 4/4] powerpc: Use trap metadata to prevent double restart rather than zeroing trap
2020-04-21 2:19 [PATCH 0/4] powerpc: clean up pt_regs traps handling Nicholas Piggin
` (2 preceding siblings ...)
2020-04-21 2:19 ` [PATCH 3/4] powerpc: TRAP_IS_SYSCALL helper to hide syscall trap number Nicholas Piggin
@ 2020-04-21 2:19 ` Nicholas Piggin
3 siblings, 0 replies; 5+ messages in thread
From: Nicholas Piggin @ 2020-04-21 2:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
It's not very nice to zero trap for this, because then system calls no
longer have TRAP_IS_SYSCALL(regs) invariant, and we can't distinguish
between sc and scv system calls (in a later patch).
Take one last unused bit from the low bits of the pt_regs.trap word for
this instead. There is not a really good reason why it should be in trap
as opposed to another field, but trap has some concept of flags and it
exists. Ideally I think we would move trap to 2-byte field and have 2
more bytes available independently.
Add a selftests case for this, which can be seen to fail if
TRAP_NORESTART is defined to false.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/ptrace.h | 15 +-
arch/powerpc/kernel/signal.c | 7 +-
arch/powerpc/kernel/signal_32.c | 2 +-
arch/powerpc/kernel/signal_64.c | 10 +-
.../testing/selftests/powerpc/signal/Makefile | 2 +-
.../powerpc/signal/sig_sc_double_restart.c | 174 ++++++++++++++++++
6 files changed, 194 insertions(+), 16 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/signal/sig_sc_double_restart.c
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 5eb249c725bd..5ee7eb128fb9 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -184,13 +184,13 @@ extern int ptrace_put_reg(struct task_struct *task, int regno,
#ifdef __powerpc64__
#ifdef CONFIG_PPC_BOOK3S
-#define TRAP(regs) ((regs)->trap)
-#define SET_TRAP(regs, val) ((regs)->trap = (val))
+#define TRAP(regs) ((regs)->trap & ~0x10)
+#define SET_TRAP(regs, val) ((regs)->trap = ((regs)->trap & 0x10) | ((val) & ~0x10))
#define FULL_REGS(regs) true
#define SET_FULL_REGS(regs) do { } while (0)
#else
-#define TRAP(regs) ((regs)->trap & ~0x1)
-#define SET_TRAP(regs, val) ((regs)->trap = ((regs)->trap & 0x1) | ((val) & ~0x1))
+#define TRAP(regs) ((regs)->trap & ~0x11)
+#define SET_TRAP(regs, val) ((regs)->trap = ((regs)->trap & 0x11) | ((val) & ~0x11))
#define FULL_REGS(regs) (((regs)->trap & 1) == 0)
#define SET_FULL_REGS(regs) ((regs)->trap |= 1)
#endif
@@ -204,8 +204,8 @@ extern int ptrace_put_reg(struct task_struct *task, int regno,
* On 4xx we use the next bit to indicate whether the exception
* is a critical exception (1 means it is).
*/
-#define TRAP(regs) ((regs)->trap & ~0xF)
-#define SET_TRAP(regs, val) ((regs)->trap = ((regs)->trap & 0xF) | ((val) & ~0xF))
+#define TRAP(regs) ((regs)->trap & ~0x1F)
+#define SET_TRAP(regs, val) ((regs)->trap = ((regs)->trap & 0x1F) | ((val) & ~0x1F))
#define FULL_REGS(regs) (((regs)->trap & 1) == 0)
#define SET_FULL_REGS(regs) ((regs)->trap |= 1)
#define IS_CRITICAL_EXC(regs) (((regs)->trap & 2) != 0)
@@ -219,6 +219,9 @@ do { \
} while (0)
#endif /* __powerpc64__ */
+#define TRAP_NORESTART(regs) (((regs)->trap & 16) == 16)
+#define SET_TRAP_NORESTART(regs) ((regs)->trap |= 16)
+
#define arch_has_single_step() (1)
#ifndef CONFIG_BOOK3S_601
#define arch_has_block_step() (true)
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index 4b74eef1d881..0de314075a8f 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -201,6 +201,9 @@ static void check_syscall_restart(struct pt_regs *regs, struct k_sigaction *ka,
if (!TRAP_IS_SYSCALL(regs))
return;
+ if (TRAP_NORESTART(regs))
+ return;
+
/* error signalled ? */
if (!(regs->ccr & 0x10000000))
return;
@@ -258,7 +261,7 @@ static void do_signal(struct task_struct *tsk)
if (ksig.sig <= 0) {
/* No signal to deliver -- put the saved sigmask back */
restore_saved_sigmask();
- tsk->thread.regs->trap = 0;
+ SET_TRAP_NORESTART(tsk->thread.regs);
return; /* no signals delivered */
}
@@ -285,7 +288,7 @@ static void do_signal(struct task_struct *tsk)
ret = handle_rt_signal64(&ksig, oldset, tsk);
}
- tsk->thread.regs->trap = 0;
+ SET_TRAP_NORESTART(tsk->thread.regs);
signal_setup_done(ret, &ksig, test_thread_flag(TIF_SINGLESTEP));
}
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 4f96d29a22bf..2970e1fd8d02 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -500,7 +500,7 @@ static long restore_user_regs(struct pt_regs *regs,
if (!sig)
save_r2 = (unsigned int)regs->gpr[2];
err = restore_general_regs(regs, sr);
- regs->trap = 0;
+ SET_TRAP_NORESTART(regs);
err |= __get_user(msr, &sr->mc_gregs[PT_MSR]);
if (!sig)
regs->gpr[2] = (unsigned long) save_r2;
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index adfde59cf4ba..1f6565dbe248 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -350,8 +350,8 @@ static long restore_sigcontext(struct task_struct *tsk, sigset_t *set, int sig,
err |= __get_user(regs->link, &sc->gp_regs[PT_LNK]);
err |= __get_user(regs->xer, &sc->gp_regs[PT_XER]);
err |= __get_user(regs->ccr, &sc->gp_regs[PT_CCR]);
- /* skip SOFTE */
- regs->trap = 0;
+ /* Don't allow userspace to set SOFTE */
+ SET_TRAP_NORESTART(regs);
err |= __get_user(regs->dar, &sc->gp_regs[PT_DAR]);
err |= __get_user(regs->dsisr, &sc->gp_regs[PT_DSISR]);
err |= __get_user(regs->result, &sc->gp_regs[PT_RESULT]);
@@ -472,10 +472,8 @@ static long restore_tm_sigcontexts(struct task_struct *tsk,
&sc->gp_regs[PT_XER]);
err |= __get_user(tsk->thread.ckpt_regs.ccr,
&sc->gp_regs[PT_CCR]);
-
- /* Don't allow userspace to set the trap value */
- regs->trap = 0;
-
+ /* Don't allow userspace to set SOFTE */
+ SET_TRAP_NORESTART(regs);
/* These regs are not checkpointed; they can go in 'regs'. */
err |= __get_user(regs->dar, &sc->gp_regs[PT_DAR]);
err |= __get_user(regs->dsisr, &sc->gp_regs[PT_DSISR]);
diff --git a/tools/testing/selftests/powerpc/signal/Makefile b/tools/testing/selftests/powerpc/signal/Makefile
index 932a032bf036..d6ae54663aed 100644
--- a/tools/testing/selftests/powerpc/signal/Makefile
+++ b/tools/testing/selftests/powerpc/signal/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-TEST_GEN_PROGS := signal signal_tm sigfuz sigreturn_vdso
+TEST_GEN_PROGS := signal signal_tm sigfuz sigreturn_vdso sig_sc_double_restart
CFLAGS += -maltivec
$(OUTPUT)/signal_tm: CFLAGS += -mhtm
diff --git a/tools/testing/selftests/powerpc/signal/sig_sc_double_restart.c b/tools/testing/selftests/powerpc/signal/sig_sc_double_restart.c
new file mode 100644
index 000000000000..64732adf3d91
--- /dev/null
+++ b/tools/testing/selftests/powerpc/signal/sig_sc_double_restart.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Test that a syscall does not get restarted twice.
+ *
+ * Based on Al's description, and a test for the bug fixed in this commit:
+ *
+ * commit 9a81c16b527528ad307843be5571111aa8d35a80
+ * Author: Al Viro <viro@zeniv.linux.org.uk>
+ * Date: Mon Sep 20 21:48:57 2010 +0100
+ *
+ * powerpc: fix double syscall restarts
+ *
+ * Make sigreturn zero regs->trap, make do_signal() do the same on all
+ * paths. As it is, signal interrupting e.g. read() from fd 512 (==
+ * ERESTARTSYS) with another signal getting unblocked when the first
+ * handler finishes will lead to restart one insn earlier than it ought
+ * to. Same for multiple signals with in-kernel handlers interrupting
+ * that sucker at the same time. Same for multiple signals of any kind
+ * interrupting that sucker on 64bit...
+ */
+#define _GNU_SOURCE
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+#include <signal.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "utils.h"
+
+static void SIGUSR1_handler(int sig)
+{
+ kill(getpid(), SIGUSR2);
+ /*
+ * SIGUSR2 is blocked until the handler exits, at which point it will
+ * be raised again and think there is a restart to be done because the
+ * pending restarted syscall has 512 (ERESTARTSYS) in r3. The second
+ * restart will retreat NIP another 4 bytes to fail case branch.
+ */
+}
+
+static void SIGUSR2_handler(int sig)
+{
+}
+
+static ssize_t raw_read(int fd, void *buf, size_t count)
+{
+ register long nr asm("r0") = __NR_read;
+ register long _fd asm("r3") = fd;
+ register void *_buf asm("r4") = buf;
+ register size_t _count asm("r5") = count;
+
+ asm volatile(
+" b 0f \n"
+" b 1f \n"
+" 0: sc 0 \n"
+" bns 2f \n"
+" neg %0,%0 \n"
+" b 2f \n"
+" 1: \n"
+" li %0,%4 \n"
+" 2: \n"
+ : "+r"(_fd), "+r"(nr), "+r"(_buf), "+r"(_count)
+ : "i"(-ENOANO)
+ : "memory", "r6", "r7", "r8", "r9", "r10", "r11", "r12", "ctr", "cr0");
+
+ if (_fd < 0) {
+ errno = -_fd;
+ _fd = -1;
+ }
+
+ return _fd;
+}
+
+#define DATA "test 123"
+#define DLEN (strlen(DATA)+1)
+
+int test_restart(void)
+{
+ int pipefd[2];
+ pid_t pid;
+ char buf[512];
+
+ if (pipe(pipefd) == -1) {
+ perror("pipe");
+ exit(EXIT_FAILURE);
+ }
+
+ pid = fork();
+ if (pid == -1) {
+ perror("fork");
+ exit(EXIT_FAILURE);
+ }
+
+ if (pid == 0) { /* Child reads from pipe */
+ struct sigaction act;
+ int fd;
+
+ memset(&act, 0, sizeof(act));
+ sigaddset(&act.sa_mask, SIGUSR2);
+ act.sa_handler = SIGUSR1_handler;
+ act.sa_flags = SA_RESTART;
+ if (sigaction(SIGUSR1, &act, NULL) == -1) {
+ perror("sigaction");
+ exit(EXIT_FAILURE);
+ }
+
+ memset(&act, 0, sizeof(act));
+ act.sa_handler = SIGUSR2_handler;
+ act.sa_flags = SA_RESTART;
+ if (sigaction(SIGUSR2, &act, NULL) == -1) {
+ perror("sigaction");
+ exit(EXIT_FAILURE);
+ }
+
+ /* Let's get ERESTARTSYS into r3 */
+ while ((fd = dup(pipefd[0])) != 512) {
+ if (fd == -1) {
+ perror("dup");
+ exit(EXIT_FAILURE);
+ }
+ }
+
+ if (raw_read(fd, buf, 512) == -1) {
+ if (errno == ENOANO) {
+ fprintf(stderr, "Double restart moved restart before sc instruction.\n");
+ _exit(EXIT_FAILURE);
+ }
+ perror("read");
+ exit(EXIT_FAILURE);
+ }
+
+ if (strncmp(buf, DATA, DLEN)) {
+ fprintf(stderr, "bad test string %s\n", buf);
+ exit(EXIT_FAILURE);
+ }
+
+ return 0;
+
+ } else {
+ int wstatus;
+
+ usleep(100000); /* Hack to get reader waiting */
+ kill(pid, SIGUSR1);
+ usleep(100000);
+ if (write(pipefd[1], DATA, DLEN) != DLEN) {
+ perror("write");
+ exit(EXIT_FAILURE);
+ }
+ close(pipefd[0]);
+ close(pipefd[1]);
+ if (wait(&wstatus) == -1) {
+ perror("wait");
+ exit(EXIT_FAILURE);
+ }
+ if (!WIFEXITED(wstatus)) {
+ fprintf(stderr, "child exited abnormally\n");
+ exit(EXIT_FAILURE);
+ }
+
+ FAIL_IF(WEXITSTATUS(wstatus) != EXIT_SUCCESS);
+
+ return 0;
+ }
+}
+
+int main(void)
+{
+ test_harness_set_timeout(10);
+ return test_harness(test_restart, "sig sys restart");
+}
--
2.23.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-04-21 2:31 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-21 2:19 [PATCH 0/4] powerpc: clean up pt_regs traps handling Nicholas Piggin
2020-04-21 2:19 ` [PATCH 1/4] powerpc/64s: Always has full regs, so remove remnant checks Nicholas Piggin
2020-04-21 2:19 ` [PATCH 2/4] powerpc: Use SET_TRAP and avoid open-coding trap masking Nicholas Piggin
2020-04-21 2:19 ` [PATCH 3/4] powerpc: TRAP_IS_SYSCALL helper to hide syscall trap number Nicholas Piggin
2020-04-21 2:19 ` [PATCH 4/4] powerpc: Use trap metadata to prevent double restart rather than zeroing trap Nicholas Piggin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).