Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0f4e6e75 authored by Will Deacon's avatar Will Deacon Committed by Greg Kroah-Hartman
Browse files

arm64: ptrace: Override SPSR.SS when single-stepping is enabled 

commit 3a5a4366cecc25daa300b9a9174f7fdd352b9068 upstream.

Luis reports that, when reverse debugging with GDB, single-step does not
function as expected on arm64:

  | I've noticed, under very specific conditions, that a PTRACE_SINGLESTEP
  | request by GDB won't execute the underlying instruction. As a consequence,
  | the PC doesn't move, but we return a SIGTRAP just like we would for a
  | regular successful PTRACE_SINGLESTEP request.

The underlying problem is that when the CPU register state is restored
as part of a reverse step, the SPSR.SS bit is cleared and so the hardware
single-step state can transition to the "active-pending" state, causing
an unexpected step exception to be taken immediately if a step operation
is attempted.

In hindsight, we probably shouldn't have exposed SPSR.SS in the pstate
accessible by the GPR regset, but it's a bit late for that now. Instead,
simply prevent userspace from configuring the bit to a value which is
inconsistent with the TIF_SINGLESTEP state for the task being traced.

Cc: <stable@vger.kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Keno Fischer <keno@juliacomputing.com>
Link: https://lore.kernel.org/r/1eed6d69-d53d-9657-1fc9-c089be07f98c@linaro.org


Reported-by: default avatarLuis Machado <luis.machado@linaro.org>
Tested-by: default avatarLuis Machado <luis.machado@linaro.org>
Signed-off-by: default avatarWill Deacon <will@kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent febe56f2

arch/arm64/include/asm/debug-monitors.h

+2 −0
Original line number Diff line number Diff line
@@ -119,6 +119,8 @@ void disable_debug_monitors(enum dbg_active_el el); 


void user_rewind_single_step(struct task_struct *task);

void user_fastforward_single_step(struct task_struct *task);

void user_regs_reset_single_step(struct user_pt_regs *regs,

				 struct task_struct *task);



void kernel_enable_single_step(struct pt_regs *regs);

void kernel_disable_single_step(void);

arch/arm64/kernel/debug-monitors.c

+16 −4
Original line number Diff line number Diff line
@@ -152,17 +152,20 @@ postcore_initcall(debug_monitors_init); 
/*

 * Single step API and exception handling.

 */

static void set_regs_spsr_ss(struct pt_regs *regs)

static void set_user_regs_spsr_ss(struct user_pt_regs *regs)

{

	regs->pstate |= DBG_SPSR_SS;

}

NOKPROBE_SYMBOL(set_regs_spsr_ss);

NOKPROBE_SYMBOL(set_user_regs_spsr_ss);



static void clear_regs_spsr_ss(struct pt_regs *regs)

static void clear_user_regs_spsr_ss(struct user_pt_regs *regs)

{

	regs->pstate &= ~DBG_SPSR_SS;

}

NOKPROBE_SYMBOL(clear_regs_spsr_ss);

NOKPROBE_SYMBOL(clear_user_regs_spsr_ss);



#define set_regs_spsr_ss(r)	set_user_regs_spsr_ss(&(r)->user_regs)

#define clear_regs_spsr_ss(r)	clear_user_regs_spsr_ss(&(r)->user_regs)



/* EL1 Single Step Handler hooks */

static LIST_HEAD(step_hook);
@@ -400,6 +403,15 @@ void user_fastforward_single_step(struct task_struct *task) 
		clear_regs_spsr_ss(task_pt_regs(task));

}



void user_regs_reset_single_step(struct user_pt_regs *regs,

				 struct task_struct *task)

{

	if (test_tsk_thread_flag(task, TIF_SINGLESTEP))

		set_user_regs_spsr_ss(regs);

	else

		clear_user_regs_spsr_ss(regs);

}



/* Kernel API */

void kernel_enable_single_step(struct pt_regs *regs)

{

arch/arm64/kernel/ptrace.c

+2 −2
Original line number Diff line number Diff line
@@ -1758,8 +1758,8 @@ static int valid_native_regs(struct user_pt_regs *regs) 
 */

int valid_user_regs(struct user_pt_regs *regs, struct task_struct *task)

{

	if (!test_tsk_thread_flag(task, TIF_SINGLESTEP))

		regs->pstate &= ~DBG_SPSR_SS;

	/* https://lore.kernel.org/lkml/20191118131525.GA4180@willie-the-truck */

	user_regs_reset_single_step(regs, task);



	if (is_compat_thread(task_thread_info(task)))

		return valid_compat_regs(regs);