git: 598e53b3d39c - stable/13 - Stop single stepping in signal handers on arm64
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 22 Feb 2022 16:40:26 UTC
The branch stable/13 has been updated by andrew: URL: https://cgit.FreeBSD.org/src/commit/?id=598e53b3d39c5b8453151080db3ccc263a04330f commit 598e53b3d39c5b8453151080db3ccc263a04330f Author: Andrew Turner <andrew@FreeBSD.org> AuthorDate: 2022-01-26 14:25:48 +0000 Commit: Andrew Turner <andrew@FreeBSD.org> CommitDate: 2022-02-22 16:23:07 +0000 Stop single stepping in signal handers on arm64 We should clear the single step flag when entering a signal hander and set it when returning. This fixes the ptrace__PT_STEP_with_signal test. While here add support for userspace to set the single step bit as on x86. This can be used by userspace for self tracing. Reviewed by: kib Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D34170 (cherry picked from commit 31cf95cec738bdb51a652689e2f829addc3b984b) --- sys/arm64/arm64/exec_machdep.c | 39 +++++++++++++++++++--- sys/arm64/arm64/freebsd32_machdep.c | 30 ++++++++++++++++- sys/arm64/arm64/ptrace_machdep.c | 12 ++++--- sys/arm64/arm64/trap.c | 12 ++++--- sys/arm64/include/armreg.h | 3 ++ tests/sys/kern/basic_signal.c | 66 +++++++++++++++++++++++++++++++++++++ 6 files changed, 148 insertions(+), 14 deletions(-) diff --git a/sys/arm64/arm64/exec_machdep.c b/sys/arm64/arm64/exec_machdep.c index a00d616a6663..6a7fc64734e1 100644 --- a/sys/arm64/arm64/exec_machdep.c +++ b/sys/arm64/arm64/exec_machdep.c @@ -97,7 +97,6 @@ set_regs(struct thread *td, struct reg *regs) frame = td->td_frame; frame->tf_sp = regs->sp; frame->tf_lr = regs->lr; - frame->tf_spsr &= ~PSR_FLAGS; memcpy(frame->tf_x, regs->x, sizeof(frame->tf_x)); @@ -109,12 +108,27 @@ set_regs(struct thread *td, struct reg *regs) * it put it. */ frame->tf_elr = regs->x[15]; - frame->tf_spsr |= regs->x[16] & PSR_FLAGS; + frame->tf_spsr &= ~PSR_SETTABLE_32; + frame->tf_spsr |= regs->x[16] & PSR_SETTABLE_32; + /* Don't allow userspace to ask to continue single stepping. + * The SPSR.SS field doesn't exist when the EL1 is AArch32. + * As the SPSR.DIT field has moved in its place don't + * allow userspace to set the SPSR.SS field. + */ } else #endif { frame->tf_elr = regs->elr; - frame->tf_spsr |= regs->spsr & PSR_FLAGS; + frame->tf_spsr &= ~PSR_SETTABLE_64; + frame->tf_spsr |= regs->spsr & PSR_SETTABLE_64; + /* Enable single stepping if userspace asked fot it */ + if ((frame->tf_spsr & PSR_SS) != 0) { + td->td_pcb->pcb_flags |= PCB_SINGLE_STEP; + + WRITE_SPECIALREG(mdscr_el1, + READ_SPECIALREG(mdscr_el1) | MDSCR_SS); + isb(); + } } return (0); } @@ -335,8 +349,8 @@ set_regs32(struct thread *td, struct reg32 *regs) tf->tf_x[13] = regs->r_sp; tf->tf_x[14] = regs->r_lr; tf->tf_elr = regs->r_pc; - tf->tf_spsr &= ~PSR_FLAGS; - tf->tf_spsr |= regs->r_cpsr & PSR_FLAGS; + tf->tf_spsr &= ~PSR_SETTABLE_32; + tf->tf_spsr |= regs->r_cpsr & PSR_SETTABLE_32; return (0); } @@ -449,6 +463,13 @@ set_mcontext(struct thread *td, mcontext_t *mcp) tf->tf_lr = mcp->mc_gpregs.gp_lr; tf->tf_elr = mcp->mc_gpregs.gp_elr; tf->tf_spsr = mcp->mc_gpregs.gp_spsr; + if ((tf->tf_spsr & PSR_SS) != 0) { + td->td_pcb->pcb_flags |= PCB_SINGLE_STEP; + + WRITE_SPECIALREG(mdscr_el1, + READ_SPECIALREG(mdscr_el1) | MDSCR_SS); + isb(); + } set_fpcontext(td, mcp); return (0); @@ -603,6 +624,14 @@ sendsig(sig_t catcher, ksiginfo_t *ksi, sigset_t *mask) tf->tf_sp = (register_t)fp; tf->tf_lr = (register_t)p->p_sysent->sv_sigcode_base; + /* Clear the single step flag while in the signal handler */ + if ((td->td_pcb->pcb_flags & PCB_SINGLE_STEP) != 0) { + td->td_pcb->pcb_flags &= ~PCB_SINGLE_STEP; + WRITE_SPECIALREG(mdscr_el1, + READ_SPECIALREG(mdscr_el1) & ~MDSCR_SS); + isb(); + } + CTR3(KTR_SIG, "sendsig: return td=%p pc=%#x sp=%#x", td, tf->tf_elr, tf->tf_sp); diff --git a/sys/arm64/arm64/freebsd32_machdep.c b/sys/arm64/arm64/freebsd32_machdep.c index f87e4342cfbd..9d7b5effac0c 100644 --- a/sys/arm64/arm64/freebsd32_machdep.c +++ b/sys/arm64/arm64/freebsd32_machdep.c @@ -200,14 +200,34 @@ set_mcontext32(struct thread *td, mcontext32_t *mcp) { struct trapframe *tf; mcontext32_vfp_t mc_vfp; + uint32_t spsr; int i; tf = td->td_frame; + spsr = mcp->mc_gregset[16]; + /* + * There is no PSR_SS in the 32-bit kernel so ignore it if it's set + * as we will set it later if needed. + */ + if ((spsr & ~(PSR_SETTABLE_32 | PSR_SS)) != + (tf->tf_spsr & ~(PSR_SETTABLE_32 | PSR_SS))) + return (EINVAL); + + spsr &= PSR_SETTABLE_32; + spsr |= tf->tf_spsr & ~PSR_SETTABLE_32; + + if ((td->td_dbgflags & TDB_STEP) != 0) { + spsr |= PSR_SS; + td->td_pcb->pcb_flags |= PCB_SINGLE_STEP; + WRITE_SPECIALREG(mdscr_el1, + READ_SPECIALREG(mdscr_el1) | MDSCR_SS); + } + for (i = 0; i < 15; i++) tf->tf_x[i] = mcp->mc_gregset[i]; tf->tf_elr = mcp->mc_gregset[15]; - tf->tf_spsr = mcp->mc_gregset[16]; + tf->tf_spsr = spsr; #ifdef VFP if (mcp->mc_vfp_size == sizeof(mc_vfp) && mcp->mc_vfp_ptr != 0) { if (copyin((void *)(uintptr_t)mcp->mc_vfp_ptr, &mc_vfp, @@ -408,6 +428,14 @@ freebsd32_sendsig(sig_t catcher, ksiginfo_t *ksi, sigset_t *mask) else tf->tf_spsr &= ~PSR_T; + /* Clear the single step flag while in the signal handler */ + if ((td->td_pcb->pcb_flags & PCB_SINGLE_STEP) != 0) { + td->td_pcb->pcb_flags &= ~PCB_SINGLE_STEP; + WRITE_SPECIALREG(mdscr_el1, + READ_SPECIALREG(mdscr_el1) & ~MDSCR_SS); + isb(); + } + CTR3(KTR_SIG, "sendsig: return td=%p pc=%#x sp=%#x", td, tf->tf_x[14], tf->tf_x[13]); diff --git a/sys/arm64/arm64/ptrace_machdep.c b/sys/arm64/arm64/ptrace_machdep.c index dd49aaa4fcec..144ff29aff47 100644 --- a/sys/arm64/arm64/ptrace_machdep.c +++ b/sys/arm64/arm64/ptrace_machdep.c @@ -58,17 +58,21 @@ ptrace_set_pc(struct thread *td, u_long addr) int ptrace_single_step(struct thread *td) { - - td->td_frame->tf_spsr |= PSR_SS; - td->td_pcb->pcb_flags |= PCB_SINGLE_STEP; + PROC_LOCK_ASSERT(td->td_proc, MA_OWNED); + if ((td->td_frame->tf_spsr & PSR_SS) == 0) { + td->td_frame->tf_spsr |= PSR_SS; + td->td_pcb->pcb_flags |= PCB_SINGLE_STEP; + td->td_dbgflags |= TDB_STEP; + } return (0); } int ptrace_clear_single_step(struct thread *td) { - + PROC_LOCK_ASSERT(td->td_proc, MA_OWNED); td->td_frame->tf_spsr &= ~PSR_SS; td->td_pcb->pcb_flags &= ~PCB_SINGLE_STEP; + td->td_dbgflags &= ~TDB_STEP; return (0); } diff --git a/sys/arm64/arm64/trap.c b/sys/arm64/arm64/trap.c index 6524679a64ba..a8d29ffaa9a7 100644 --- a/sys/arm64/arm64/trap.c +++ b/sys/arm64/arm64/trap.c @@ -604,10 +604,14 @@ do_el0_sync(struct thread *td, struct trapframe *frame) userret(td, frame); break; case EXCP_SOFTSTP_EL0: - td->td_frame->tf_spsr &= ~PSR_SS; - td->td_pcb->pcb_flags &= ~PCB_SINGLE_STEP; - WRITE_SPECIALREG(mdscr_el1, - READ_SPECIALREG(mdscr_el1) & ~MDSCR_SS); + PROC_LOCK(td->td_proc); + if ((td->td_dbgflags & TDB_STEP) != 0) { + td->td_frame->tf_spsr &= ~PSR_SS; + td->td_pcb->pcb_flags &= ~PCB_SINGLE_STEP; + WRITE_SPECIALREG(mdscr_el1, + READ_SPECIALREG(mdscr_el1) & ~MDSCR_SS); + } + PROC_UNLOCK(td->td_proc); call_trapsignal(td, SIGTRAP, TRAP_TRACE, (void *)frame->tf_elr, exception); userret(td, frame); diff --git a/sys/arm64/include/armreg.h b/sys/arm64/include/armreg.h index 773f0b8d9402..1404ca8cd727 100644 --- a/sys/arm64/include/armreg.h +++ b/sys/arm64/include/armreg.h @@ -1312,6 +1312,9 @@ #define PSR_Z 0x40000000 #define PSR_N 0x80000000 #define PSR_FLAGS 0xf0000000 +/* PSR fields that can be set from 32-bit and 64-bit processes */ +#define PSR_SETTABLE_32 PSR_FLAGS +#define PSR_SETTABLE_64 (PSR_FLAGS | PSR_SS) /* TCR_EL1 - Translation Control Register */ /* Bits 63:59 are reserved */ diff --git a/tests/sys/kern/basic_signal.c b/tests/sys/kern/basic_signal.c index b94ebeb1ba23..1dcb9ff064fc 100644 --- a/tests/sys/kern/basic_signal.c +++ b/tests/sys/kern/basic_signal.c @@ -10,6 +10,20 @@ #include <stdbool.h> #include <stdlib.h> +#if defined(__aarch64__) +#include <machine/armreg.h> +#define SET_TRACE_FLAG(ucp) (ucp)->uc_mcontext.mc_gpregs.gp_spsr |= PSR_SS +#define CLR_TRACE_FLAG(ucp) (ucp)->uc_mcontext.mc_gpregs.gp_spsr &= ~PSR_SS +#elif defined(__amd64__) +#include <machine/psl.h> +#define SET_TRACE_FLAG(ucp) (ucp)->uc_mcontext.mc_rflags |= PSL_T +#define CLR_TRACE_FLAG(ucp) (ucp)->uc_mcontext.mc_rflags &= ~PSL_T +#elif defined(__i386__) +#include <machine/psl.h> +#define SET_TRACE_FLAG(ucp) (ucp)->uc_mcontext.mc_eflags |= PSL_T +#define CLR_TRACE_FLAG(ucp) (ucp)->uc_mcontext.mc_eflags &= ~PSL_T +#endif + static volatile sig_atomic_t signal_fired = 0; static void @@ -62,6 +76,55 @@ ATF_TC_BODY(signal_test, tc) ATF_CHECK(signal_fired == 3); } +/* + * Check setting the machine dependent single step flag works when supported. + */ +#ifdef SET_TRACE_FLAG +static volatile sig_atomic_t trap_signal_fired = 0; + +static void +trap_sig_handler(int signo, siginfo_t *info __unused, void *_ucp) +{ + ucontext_t *ucp = _ucp; + + if (trap_signal_fired < 9) { + SET_TRACE_FLAG(ucp); + } else { + CLR_TRACE_FLAG(ucp); + } + trap_signal_fired++; +} + +ATF_TC(trap_signal_test); + +ATF_TC_HEAD(trap_signal_test, tc) +{ + + atf_tc_set_md_var(tc, "descr", + "Testing signal handler setting the MD single step flag"); +} + +ATF_TC_BODY(trap_signal_test, tc) +{ + /* + * Setup the signal handlers + */ + struct sigaction sa = { + .sa_sigaction = trap_sig_handler, + .sa_flags = SA_SIGINFO, + }; + ATF_REQUIRE(sigemptyset(&sa.sa_mask) == 0); + ATF_REQUIRE(sigaction(SIGTRAP, &sa, NULL) == 0); + + /* + * Fire SIGTRAP + */ + ATF_CHECK(trap_signal_fired == 0); + ATF_REQUIRE(raise(SIGTRAP) == 0); + ATF_CHECK(trap_signal_fired == 10); +} +#endif + /* * Special tests for 32-bit arm. We can call thumb code (really just t32) from * normal (a32) mode and vice versa. Likewise, signals can interrupt a T32 @@ -150,6 +213,9 @@ ATF_TP_ADD_TCS(tp) { ATF_TP_ADD_TC(tp, signal_test); +#ifdef SET_TRACE_FLAG + ATF_TP_ADD_TC(tp, trap_signal_test); +#endif #ifdef __arm__ ATF_TP_ADD_TC(tp, signal_test_T32_to_A32); ATF_TP_ADD_TC(tp, signal_test_A32_to_T32);