Re: [PATCH 2/3] arm64: signal: sigreturn() and rt_sigreturn() sometime returns the wrong signals
From: Will Deacon <will@kernel.org>
Date: 2021-04-22 12:49:01
Also in:
bpf, lkml, netdev
[+Eric as he actually understands how this is supposed to work] On Tue, Apr 20, 2021 at 04:50:13PM +0000, Liam Howlett wrote:
quoted hunk ↗ jump to hunk
arm64_notify_segfault() was used to force a SIGSEGV in all error cases in sigreturn() and rt_sigreturn() to avoid writing a new sig handler. There is now a better sig handler to use which does not search the VMA address space and return a slightly incorrect error code. Restore the older and correct si_code of SI_KERNEL by using arm64_notify_die(). In the case of !access_ok(), simply return SIGSEGV with si_code SEGV_ACCERR. This change requires exporting arm64_notfiy_die() to the arm64 traps.h Fixes: f71016a8a8c5 (arm64: signal: Call arm64_notify_segfault when failing to deliver signal) Signed-off-by: Liam R. Howlett <redacted> --- arch/arm64/include/asm/traps.h | 2 ++ arch/arm64/kernel/signal.c | 8 ++++++-- arch/arm64/kernel/signal32.c | 18 ++++++++++++++---- 3 files changed, 22 insertions(+), 6 deletions(-)diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h index 54f32a0675df..9b76144fcba6 100644 --- a/arch/arm64/include/asm/traps.h +++ b/arch/arm64/include/asm/traps.h@@ -29,6 +29,8 @@ void arm64_notify_segfault(unsigned long addr); void arm64_force_sig_fault(int signo, int code, unsigned long far, const char *str); void arm64_force_sig_mceerr(int code, unsigned long far, short lsb, const char *str); void arm64_force_sig_ptrace_errno_trap(int errno, unsigned long far, const char *str); +void arm64_notify_die(const char *str, struct pt_regs *regs, int signo, + int sicode, unsigned long far, int err); /* * Move regs->pc to next instruction and do necessary setup before itdiff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c index 6237486ff6bb..9fde6dc760c3 100644 --- a/arch/arm64/kernel/signal.c +++ b/arch/arm64/kernel/signal.c@@ -544,7 +544,7 @@ SYSCALL_DEFINE0(rt_sigreturn) frame = (struct rt_sigframe __user *)regs->sp; if (!access_ok(frame, sizeof (*frame))) - goto badframe; + goto e_access; if (restore_sigframe(regs, frame)) goto badframe;@@ -555,7 +555,11 @@ SYSCALL_DEFINE0(rt_sigreturn) return regs->regs[0]; badframe: - arm64_notify_segfault(regs->sp); + arm64_notify_die("Bad frame", regs, SIGSEGV, SI_KERNEL, regs->sp, 0); + return 0; + +e_access: + force_signal_inject(SIGSEGV, SEGV_ACCERR, regs->sp, 0); return 0;
This seems really error-prone to me, but maybe I'm just missing some context. What's the rule for reporting an si_code of SI_KERNEL vs SEGV_ACCERR, and is the former actually valid for SIGSEGV? With this change, pointing the (signal) stack to a kernel address will result in SEGV_ACCERR but pointing it to something like a PROT_NONE user address will give SI_KERNEL (well, assuming that we manage to deliver the SEGV somehow). I'm having a hard time seeing why that's a useful distinction to make.. If it's important to get this a particular way around, please can you add some selftests? Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel