Commit 26a04d84 authored by Will Deacon's avatar Will Deacon
Browse files

arm64: debug: Separate debug hooks based on target exception level 



Mixing kernel and user debug hooks together is highly error-prone as it
relies on all of the hooks to figure out whether the exception came from
kernel or user, and then to act accordingly.

Make our debug hook code a little more robust by maintaining separate
hook lists for user and kernel, with separate registration functions
to force callers to be explicit about the exception levels that they
care about.

Reviewed-by: default avatarMark Rutland <mark.rutland@arm.com>
Signed-off-by: default avatarWill Deacon <will.deacon@arm.com>
parent cb764a69

arch/arm64/include/asm/brk-imm.h

+1 −0
Original line number Diff line number Diff line
@@ -23,5 +23,6 @@ 
#define KGDB_COMPILED_DBG_BRK_IMM	0x401

#define BUG_BRK_IMM			0x800

#define KASAN_BRK_IMM			0x900

#define KASAN_BRK_MASK			0x0ff



#endif

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

+12 −6
Original line number Diff line number Diff line
@@ -94,18 +94,24 @@ struct step_hook { 
	int (*fn)(struct pt_regs *regs, unsigned int esr);

};



void register_step_hook(struct step_hook *hook);

void unregister_step_hook(struct step_hook *hook);

void register_user_step_hook(struct step_hook *hook);

void unregister_user_step_hook(struct step_hook *hook);



void register_kernel_step_hook(struct step_hook *hook);

void unregister_kernel_step_hook(struct step_hook *hook);



struct break_hook {

	struct list_head node;

	u32 esr_val;

	u32 esr_mask;

	int (*fn)(struct pt_regs *regs, unsigned int esr);

	u16 imm;

	u16 mask; /* These bits are ignored when comparing with imm */

};



void register_break_hook(struct break_hook *hook);

void unregister_break_hook(struct break_hook *hook);

void register_user_break_hook(struct break_hook *hook);

void unregister_user_break_hook(struct break_hook *hook);



void register_kernel_break_hook(struct break_hook *hook);

void unregister_kernel_break_hook(struct break_hook *hook);



u8 debug_monitors_arch(void);

arch/arm64/kernel/debug-monitors.c

+60 −25
Original line number Diff line number Diff line
@@ -163,25 +163,46 @@ static void clear_regs_spsr_ss(struct pt_regs *regs) 
}

NOKPROBE_SYMBOL(clear_regs_spsr_ss);



/* EL1 Single Step Handler hooks */

static LIST_HEAD(step_hook);

static DEFINE_SPINLOCK(step_hook_lock);

static DEFINE_SPINLOCK(debug_hook_lock);

static LIST_HEAD(user_step_hook);

static LIST_HEAD(kernel_step_hook);



void register_step_hook(struct step_hook *hook)

static void register_debug_hook(struct list_head *node, struct list_head *list)

{

	spin_lock(&step_hook_lock);

	list_add_rcu(&hook->node, &step_hook);

	spin_unlock(&step_hook_lock);

	spin_lock(&debug_hook_lock);

	list_add_rcu(node, list);

	spin_unlock(&debug_hook_lock);



}



void unregister_step_hook(struct step_hook *hook)

static void unregister_debug_hook(struct list_head *node)

{

	spin_lock(&step_hook_lock);

	list_del_rcu(&hook->node);

	spin_unlock(&step_hook_lock);

	spin_lock(&debug_hook_lock);

	list_del_rcu(node);

	spin_unlock(&debug_hook_lock);

	synchronize_rcu();

}



void register_user_step_hook(struct step_hook *hook)

{

	register_debug_hook(&hook->node, &user_step_hook);

}



void unregister_user_step_hook(struct step_hook *hook)

{

	unregister_debug_hook(&hook->node);

}



void register_kernel_step_hook(struct step_hook *hook)

{

	register_debug_hook(&hook->node, &kernel_step_hook);

}



void unregister_kernel_step_hook(struct step_hook *hook)

{

	unregister_debug_hook(&hook->node);

}



/*

 * Call registered single step handlers

 * There is no Syndrome info to check for determining the handler.
@@ -191,11 +212,14 @@ void unregister_step_hook(struct step_hook *hook) 
static int call_step_hook(struct pt_regs *regs, unsigned int esr)

{

	struct step_hook *hook;

	struct list_head *list;

	int retval = DBG_HOOK_ERROR;



	list = user_mode(regs) ? &user_step_hook : &kernel_step_hook;



	rcu_read_lock();



	list_for_each_entry_rcu(hook, &step_hook, node)	{

	list_for_each_entry_rcu(hook, list, node)	{

		retval = hook->fn(regs, esr);

		if (retval == DBG_HOOK_HANDLED)

			break;
@@ -264,33 +288,44 @@ static int single_step_handler(unsigned long unused, unsigned int esr, 
}

NOKPROBE_SYMBOL(single_step_handler);



static LIST_HEAD(break_hook);

static DEFINE_SPINLOCK(break_hook_lock);

static LIST_HEAD(user_break_hook);

static LIST_HEAD(kernel_break_hook);



void register_break_hook(struct break_hook *hook)

void register_user_break_hook(struct break_hook *hook)

{

	spin_lock(&break_hook_lock);

	list_add_rcu(&hook->node, &break_hook);

	spin_unlock(&break_hook_lock);

	register_debug_hook(&hook->node, &user_break_hook);

}



void unregister_break_hook(struct break_hook *hook)

void unregister_user_break_hook(struct break_hook *hook)

{

	spin_lock(&break_hook_lock);

	list_del_rcu(&hook->node);

	spin_unlock(&break_hook_lock);

	synchronize_rcu();

	unregister_debug_hook(&hook->node);

}



void register_kernel_break_hook(struct break_hook *hook)

{

	register_debug_hook(&hook->node, &kernel_break_hook);

}



void unregister_kernel_break_hook(struct break_hook *hook)

{

	unregister_debug_hook(&hook->node);

}



static int call_break_hook(struct pt_regs *regs, unsigned int esr)

{

	struct break_hook *hook;

	struct list_head *list;

	int (*fn)(struct pt_regs *regs, unsigned int esr) = NULL;



	list = user_mode(regs) ? &user_break_hook : &kernel_break_hook;



	rcu_read_lock();

	list_for_each_entry_rcu(hook, &break_hook, node)

		if ((esr & hook->esr_mask) == hook->esr_val)

	list_for_each_entry_rcu(hook, list, node) {

		unsigned int comment = esr & BRK64_ESR_MASK;



		if ((comment & ~hook->mask) == hook->imm)

			fn = hook->fn;

	}

	rcu_read_unlock();



	return fn ? fn(regs, esr) : DBG_HOOK_ERROR;

arch/arm64/kernel/kgdb.c

+10 −12
Original line number Diff line number Diff line
@@ -275,15 +275,13 @@ static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned int esr) 
NOKPROBE_SYMBOL(kgdb_step_brk_fn);



static struct break_hook kgdb_brkpt_hook = {

	.esr_mask	= 0xffffffff,

	.esr_val	= (u32)ESR_ELx_VAL_BRK64(KGDB_DYN_DBG_BRK_IMM),

	.fn		= kgdb_brk_fn

	.fn		= kgdb_brk_fn,

	.imm		= KGDB_DYN_DBG_BRK_IMM,

};



static struct break_hook kgdb_compiled_brkpt_hook = {

	.esr_mask	= 0xffffffff,

	.esr_val	= (u32)ESR_ELx_VAL_BRK64(KGDB_COMPILED_DBG_BRK_IMM),

	.fn		= kgdb_compiled_brk_fn

	.fn		= kgdb_compiled_brk_fn,

	.imm		= KGDB_COMPILED_DBG_BRK_IMM,

};



static struct step_hook kgdb_step_hook = {
@@ -332,9 +330,9 @@ int kgdb_arch_init(void) 
	if (ret != 0)

		return ret;



	register_break_hook(&kgdb_brkpt_hook);

	register_break_hook(&kgdb_compiled_brkpt_hook);

	register_step_hook(&kgdb_step_hook);

	register_kernel_break_hook(&kgdb_brkpt_hook);

	register_kernel_break_hook(&kgdb_compiled_brkpt_hook);

	register_kernel_step_hook(&kgdb_step_hook);

	return 0;

}
@@ -345,9 +343,9 @@ int kgdb_arch_init(void) 
 */

void kgdb_arch_exit(void)

{

	unregister_break_hook(&kgdb_brkpt_hook);

	unregister_break_hook(&kgdb_compiled_brkpt_hook);

	unregister_step_hook(&kgdb_step_hook);

	unregister_kernel_break_hook(&kgdb_brkpt_hook);

	unregister_kernel_break_hook(&kgdb_compiled_brkpt_hook);

	unregister_kernel_step_hook(&kgdb_step_hook);

	unregister_die_notifier(&kgdb_notifier);

}

arch/arm64/kernel/probes/uprobes.c

+3 −4
Original line number Diff line number Diff line
@@ -195,8 +195,7 @@ static int uprobe_single_step_handler(struct pt_regs *regs, 


/* uprobe breakpoint handler hook */

static struct break_hook uprobes_break_hook = {

	.esr_mask = BRK64_ESR_MASK,

	.esr_val = BRK64_ESR_UPROBES,

	.imm = BRK64_ESR_UPROBES,

	.fn = uprobe_breakpoint_handler,

};
@@ -207,8 +206,8 @@ static struct step_hook uprobes_step_hook = { 


static int __init arch_init_uprobes(void)

{

	register_break_hook(&uprobes_break_hook);

	register_step_hook(&uprobes_step_hook);

	register_user_break_hook(&uprobes_break_hook);

	register_user_step_hook(&uprobes_step_hook);



	return 0;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE