* [PATCH v2 1/4] powerpc/64s: Always has full regs, so remove remnant checks
@ 2020-05-07 12:13 Michael Ellerman
2020-05-07 12:13 ` [PATCH v2 2/4] powerpc: Use set_trap() and avoid open-coding trap masking Michael Ellerman
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Michael Ellerman @ 2020-05-07 12:13 UTC (permalink / raw)
To: linuxppc-dev
From: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/ptrace.h | 23 ++++++++++++++++-------
arch/powerpc/kernel/process.c | 2 +-
2 files changed, 17 insertions(+), 8 deletions(-)
v2: Unchanged.
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 8479c762aef2..8af3583546b7 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1720,7 +1720,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.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/4] powerpc: Use set_trap() and avoid open-coding trap masking
2020-05-07 12:13 [PATCH v2 1/4] powerpc/64s: Always has full regs, so remove remnant checks Michael Ellerman
@ 2020-05-07 12:13 ` Michael Ellerman
2020-05-07 12:13 ` [PATCH v2 3/4] powerpc: trap_is_syscall() helper to hide syscall trap number Michael Ellerman
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2020-05-07 12:13 UTC (permalink / raw)
To: linuxppc-dev
From: Nicholas Piggin <npiggin@gmail.com>
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>
[mpe: Make it a static inline rather than a shouty macro]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/ptrace.h | 8 ++++++++
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, 11 insertions(+), 3 deletions(-)
v2: mpe: Make it a static inline rather than a shouty macro
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 89f31d5a8062..7c585bddc06e 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -182,10 +182,12 @@ extern int ptrace_put_reg(struct task_struct *task, int regno,
#ifdef __powerpc64__
#ifdef CONFIG_PPC_BOOK3S
+#define TRAP_FLAGS_MASK 0
#define TRAP(regs) ((regs)->trap)
#define FULL_REGS(regs) true
#define SET_FULL_REGS(regs) do { } while (0)
#else
+#define TRAP_FLAGS_MASK 0x1
#define TRAP(regs) ((regs)->trap & ~0x1)
#define FULL_REGS(regs) (((regs)->trap & 1) == 0)
#define SET_FULL_REGS(regs) ((regs)->trap |= 1)
@@ -200,6 +202,7 @@ 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_FLAGS_MASK 0xF
#define TRAP(regs) ((regs)->trap & ~0xF)
#define FULL_REGS(regs) (((regs)->trap & 1) == 0)
#define SET_FULL_REGS(regs) ((regs)->trap |= 1)
@@ -214,6 +217,11 @@ do { \
} while (0)
#endif /* __powerpc64__ */
+static inline void set_trap(struct pt_regs *regs, unsigned long val)
+{
+ regs->trap = (regs->trap & TRAP_FLAGS_MASK) | (val & ~TRAP_FLAGS_MASK);
+}
+
#define arch_has_single_step() (1)
#ifndef CONFIG_BOOK3S_601
#define arch_has_block_step() (true)
diff --git a/arch/powerpc/kernel/ptrace/ptrace-tm.c b/arch/powerpc/kernel/ptrace/ptrace-tm.c
index d75aff31f637..32d62c606681 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..caeb5822a8f4 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..92761e47fb5c 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.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 3/4] powerpc: trap_is_syscall() helper to hide syscall trap number
2020-05-07 12:13 [PATCH v2 1/4] powerpc/64s: Always has full regs, so remove remnant checks Michael Ellerman
2020-05-07 12:13 ` [PATCH v2 2/4] powerpc: Use set_trap() and avoid open-coding trap masking Michael Ellerman
@ 2020-05-07 12:13 ` Michael Ellerman
2020-05-07 12:13 ` [PATCH v2 4/4] powerpc: Use trap metadata to prevent double restart rather than zeroing trap Michael Ellerman
2020-05-20 11:00 ` [PATCH v2 1/4] powerpc/64s: Always has full regs, so remove remnant checks Michael Ellerman
3 siblings, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2020-05-07 12:13 UTC (permalink / raw)
To: linuxppc-dev
From: Nicholas Piggin <npiggin@gmail.com>
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>
[mpe: Make it a static inline]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/ptrace.h | 5 +++++
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, 12 insertions(+), 4 deletions(-)
v2: mpe: Make it a static inline
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 7c585bddc06e..5db45790a087 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -222,6 +222,11 @@ static inline void set_trap(struct pt_regs *regs, unsigned long val)
regs->trap = (regs->trap & TRAP_FLAGS_MASK) | (val & ~TRAP_FLAGS_MASK);
}
+static inline bool trap_is_syscall(struct pt_regs *regs)
+{
+ return TRAP(regs) == 0xc00;
+}
+
#define arch_has_single_step() (1)
#ifndef CONFIG_BOOK3S_601
#define arch_has_block_step() (true)
diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
index 38d62acfdce7..fd1b518eed17 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 8af3583546b7..db766252238f 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1413,7 +1413,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..f2be9e960c2e 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 92761e47fb5c..a7430632bab4 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.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 4/4] powerpc: Use trap metadata to prevent double restart rather than zeroing trap
2020-05-07 12:13 [PATCH v2 1/4] powerpc/64s: Always has full regs, so remove remnant checks Michael Ellerman
2020-05-07 12:13 ` [PATCH v2 2/4] powerpc: Use set_trap() and avoid open-coding trap masking Michael Ellerman
2020-05-07 12:13 ` [PATCH v2 3/4] powerpc: trap_is_syscall() helper to hide syscall trap number Michael Ellerman
@ 2020-05-07 12:13 ` Michael Ellerman
2020-05-20 11:00 ` [PATCH v2 1/4] powerpc/64s: Always has full regs, so remove remnant checks Michael Ellerman
3 siblings, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2020-05-07 12:13 UTC (permalink / raw)
To: linuxppc-dev
From: Nicholas Piggin <npiggin@gmail.com>
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 changed to return false.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
[mpe: Make them static inlines]
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/ptrace.h | 22 ++-
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, 201 insertions(+), 16 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/signal/sig_sc_double_restart.c
v2: mpe: Make them static inlines
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 5db45790a087..b92877a81626 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -182,13 +182,13 @@ extern int ptrace_put_reg(struct task_struct *task, int regno,
#ifdef __powerpc64__
#ifdef CONFIG_PPC_BOOK3S
-#define TRAP_FLAGS_MASK 0
-#define TRAP(regs) ((regs)->trap)
+#define TRAP_FLAGS_MASK 0x10
+#define TRAP(regs) ((regs)->trap & ~TRAP_FLAGS_MASK)
#define FULL_REGS(regs) true
#define SET_FULL_REGS(regs) do { } while (0)
#else
-#define TRAP_FLAGS_MASK 0x1
-#define TRAP(regs) ((regs)->trap & ~0x1)
+#define TRAP_FLAGS_MASK 0x11
+#define TRAP(regs) ((regs)->trap & ~TRAP_FLAGS_MASK)
#define FULL_REGS(regs) (((regs)->trap & 1) == 0)
#define SET_FULL_REGS(regs) ((regs)->trap |= 1)
#endif
@@ -202,8 +202,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_FLAGS_MASK 0xF
-#define TRAP(regs) ((regs)->trap & ~0xF)
+#define TRAP_FLAGS_MASK 0x1F
+#define TRAP(regs) ((regs)->trap & ~TRAP_FLAGS_MASK)
#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)
@@ -227,6 +227,16 @@ static inline bool trap_is_syscall(struct pt_regs *regs)
return TRAP(regs) == 0xc00;
}
+static inline bool trap_norestart(struct pt_regs *regs)
+{
+ return regs->trap & 0x10;
+}
+
+static inline void set_trap_norestart(struct pt_regs *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 f2be9e960c2e..a46c3fdb6853 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..ae3da7440b2f 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..77061915897f 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.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/4] powerpc/64s: Always has full regs, so remove remnant checks
2020-05-07 12:13 [PATCH v2 1/4] powerpc/64s: Always has full regs, so remove remnant checks Michael Ellerman
` (2 preceding siblings ...)
2020-05-07 12:13 ` [PATCH v2 4/4] powerpc: Use trap metadata to prevent double restart rather than zeroing trap Michael Ellerman
@ 2020-05-20 11:00 ` Michael Ellerman
3 siblings, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2020-05-20 11:00 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
On Thu, 7 May 2020 22:13:29 +1000, Michael Ellerman wrote:
>
Applied to powerpc/next.
[1/4] powerpc/64s: Always has full regs, so remove remnant checks
https://git.kernel.org/powerpc/c/feb9df3462e688d073848d85c8bb132fe8fd9ae5
[2/4] powerpc: Use set_trap() and avoid open-coding trap masking
https://git.kernel.org/powerpc/c/db30144b5c9cfb09c6b8b2fa7a9c351c94aa3433
[3/4] powerpc: trap_is_syscall() helper to hide syscall trap number
https://git.kernel.org/powerpc/c/912237ea166428edcbf3c137adf12cb987c477f2
[4/4] powerpc: Use trap metadata to prevent double restart rather than zeroing trap
https://git.kernel.org/powerpc/c/4e0e45b07d790253643ee05300784ab2156e2d5e
cheers
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-20 11:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 12:13 [PATCH v2 1/4] powerpc/64s: Always has full regs, so remove remnant checks Michael Ellerman
2020-05-07 12:13 ` [PATCH v2 2/4] powerpc: Use set_trap() and avoid open-coding trap masking Michael Ellerman
2020-05-07 12:13 ` [PATCH v2 3/4] powerpc: trap_is_syscall() helper to hide syscall trap number Michael Ellerman
2020-05-07 12:13 ` [PATCH v2 4/4] powerpc: Use trap metadata to prevent double restart rather than zeroing trap Michael Ellerman
2020-05-20 11:00 ` [PATCH v2 1/4] powerpc/64s: Always has full regs, so remove remnant checks Michael Ellerman
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.