Skip to content

Commit

Permalink
arm64: kprobes: Use BRK instead of single-step when executing instruc…
Browse files Browse the repository at this point in the history 
…tions out-of-line

Commit 36dadef ("kprobes: Init kprobes in early_initcall") enabled
using kprobes from early_initcall. Unfortunately at this point the
hardware debug infrastructure is not operational. The OS lock may still
be locked, and the hardware watchpoints may have unknown values when
kprobe enables debug monitors to single-step instructions.

Rather than using hardware single-step, append a BRK instruction after
the instruction to be executed out-of-line.

Fixes: 36dadef ("kprobes: Init kprobes in early_initcall")
Suggested-by: Will Deacon <will@kernel.org>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Link: https://lore.kernel.org/r/20201103134900.337243-1-jean-philippe@linaro.org
Signed-off-by: Will Deacon <will@kernel.org>
  • Loading branch information
Jean-Philippe Brucker authored and Will Deacon committed Nov 3, 2020
1 parent 2a13c13 commit 7ee31a3
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 47 deletions.
2 changes: 2 additions & 0 deletions arch/arm64/include/asm/brk-imm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* #imm16 values used for BRK instruction generation
* 0x004: for installing kprobes
* 0x005: for installing uprobes
* 0x006: for kprobe software single-step
* Allowed values for kgdb are 0x400 - 0x7ff
* 0x100: for triggering a fault on purpose (reserved)
* 0x400: for dynamic BRK instruction
Expand All @@ -19,6 +20,7 @@
*/
#define KPROBES_BRK_IMM 0x004
#define UPROBES_BRK_IMM 0x005
#define KPROBES_BRK_SS_IMM 0x006
#define FAULT_BRK_IMM 0x100
#define KGDB_DYN_DBG_BRK_IMM 0x400
#define KGDB_COMPILED_DBG_BRK_IMM 0x401
Expand Down
1 change: 1 addition & 0 deletions arch/arm64/include/asm/debug-monitors.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@

/* kprobes BRK opcodes with ESR encoding */
#define BRK64_OPCODE_KPROBES (AARCH64_BREAK_MON | (KPROBES_BRK_IMM << 5))
#define BRK64_OPCODE_KPROBES_SS (AARCH64_BREAK_MON | (KPROBES_BRK_SS_IMM << 5))
/* uprobes BRK opcodes with ESR encoding */
#define BRK64_OPCODE_UPROBES (AARCH64_BREAK_MON | (UPROBES_BRK_IMM << 5))

Expand Down
2 changes: 1 addition & 1 deletion arch/arm64/include/asm/kprobes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#include <linux/percpu.h>

#define __ARCH_WANT_KPROBES_INSN_SLOT
#define MAX_INSN_SIZE 1
#define MAX_INSN_SIZE 2

#define flush_insn_slot(p) do { } while (0)
#define kretprobe_blacklist_size 0
Expand Down
69 changes: 23 additions & 46 deletions arch/arm64/kernel/probes/kprobes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,16 @@ DEFINE_PER_CPU(struct kprobe_ctlblk, kprobe_ctlblk);
static void __kprobes
post_kprobe_handler(struct kprobe_ctlblk *, struct pt_regs *);

static int __kprobes patch_text(kprobe_opcode_t *addr, u32 opcode)
{
void *addrs[1];
u32 insns[1];

addrs[0] = addr;
insns[0] = opcode;

return aarch64_insn_patch_text(addrs, insns, 1);
}

static void __kprobes arch_prepare_ss_slot(struct kprobe *p)
{
kprobe_opcode_t *addr = p->ainsn.api.insn;
void *addrs[] = {addr, addr + 1};
u32 insns[] = {p->opcode, BRK64_OPCODE_KPROBES_SS};

/* prepare insn slot */
patch_text(p->ainsn.api.insn, p->opcode);
aarch64_insn_patch_text(addrs, insns, 2);

flush_icache_range((uintptr_t) (p->ainsn.api.insn),
(uintptr_t) (p->ainsn.api.insn) +
MAX_INSN_SIZE * sizeof(kprobe_opcode_t));
flush_icache_range((uintptr_t)addr, (uintptr_t)(addr + MAX_INSN_SIZE));

/*
* Needs restoring of return address after stepping xol.
Expand Down Expand Up @@ -128,13 +119,18 @@ void *alloc_insn_page(void)
/* arm kprobe: install breakpoint in text */
void __kprobes arch_arm_kprobe(struct kprobe *p)
{
patch_text(p->addr, BRK64_OPCODE_KPROBES);
void *addr = p->addr;
u32 insn = BRK64_OPCODE_KPROBES;

aarch64_insn_patch_text(&addr, &insn, 1);
}

/* disarm kprobe: remove breakpoint from text */
void __kprobes arch_disarm_kprobe(struct kprobe *p)
{
patch_text(p->addr, p->opcode);
void *addr = p->addr;

aarch64_insn_patch_text(&addr, &p->opcode, 1);
}

void __kprobes arch_remove_kprobe(struct kprobe *p)
Expand Down Expand Up @@ -163,20 +159,15 @@ static void __kprobes set_current_kprobe(struct kprobe *p)
}

/*
* Interrupts need to be disabled before single-step mode is set, and not
* reenabled until after single-step mode ends.
* Without disabling interrupt on local CPU, there is a chance of
* interrupt occurrence in the period of exception return and start of
* out-of-line single-step, that result in wrongly single stepping
* into the interrupt handler.
* Mask all of DAIF while executing the instruction out-of-line, to keep things
* simple and avoid nesting exceptions. Interrupts do have to be disabled since
* the kprobe state is per-CPU and doesn't get migrated.
*/
static void __kprobes kprobes_save_local_irqflag(struct kprobe_ctlblk *kcb,
struct pt_regs *regs)
{
kcb->saved_irqflag = regs->pstate & DAIF_MASK;
regs->pstate |= PSR_I_BIT;
/* Unmask PSTATE.D for enabling software step exceptions. */
regs->pstate &= ~PSR_D_BIT;
regs->pstate |= DAIF_MASK;
}

static void __kprobes kprobes_restore_local_irqflag(struct kprobe_ctlblk *kcb,
Expand Down Expand Up @@ -219,10 +210,7 @@ static void __kprobes setup_singlestep(struct kprobe *p,
slot = (unsigned long)p->ainsn.api.insn;

set_ss_context(kcb, slot); /* mark pending ss */

/* IRQs and single stepping do not mix well. */
kprobes_save_local_irqflag(kcb, regs);
kernel_enable_single_step(regs);
instruction_pointer_set(regs, slot);
} else {
/* insn simulation */
Expand Down Expand Up @@ -273,12 +261,8 @@ post_kprobe_handler(struct kprobe_ctlblk *kcb, struct pt_regs *regs)
}
/* call post handler */
kcb->kprobe_status = KPROBE_HIT_SSDONE;
if (cur->post_handler) {
/* post_handler can hit breakpoint and single step
* again, so we enable D-flag for recursive exception.
*/
if (cur->post_handler)
cur->post_handler(cur, regs, 0);
}

reset_current_kprobe();
}
Expand All @@ -302,8 +286,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr)
if (!instruction_pointer(regs))
BUG();

kernel_disable_single_step();

if (kcb->kprobe_status == KPROBE_REENTER)
restore_previous_kprobe(kcb);
else
Expand Down Expand Up @@ -365,10 +347,6 @@ static void __kprobes kprobe_handler(struct pt_regs *regs)
* pre-handler and it returned non-zero, it will
* modify the execution path and no need to single
* stepping. Let's just reset current kprobe and exit.
*
* pre_handler can hit a breakpoint and can step thru
* before return, keep PSTATE D-flag enabled until
* pre_handler return back.
*/
if (!p->pre_handler || !p->pre_handler(p, regs)) {
setup_singlestep(p, regs, kcb, 0);
Expand Down Expand Up @@ -399,7 +377,7 @@ kprobe_ss_hit(struct kprobe_ctlblk *kcb, unsigned long addr)
}

static int __kprobes
kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr)
kprobe_breakpoint_ss_handler(struct pt_regs *regs, unsigned int esr)
{
struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
int retval;
Expand All @@ -409,16 +387,15 @@ kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr)

if (retval == DBG_HOOK_HANDLED) {
kprobes_restore_local_irqflag(kcb, regs);
kernel_disable_single_step();

post_kprobe_handler(kcb, regs);
}

return retval;
}

static struct step_hook kprobes_step_hook = {
.fn = kprobe_single_step_handler,
static struct break_hook kprobes_break_ss_hook = {
.imm = KPROBES_BRK_SS_IMM,
.fn = kprobe_breakpoint_ss_handler,
};

static int __kprobes
Expand Down Expand Up @@ -486,7 +463,7 @@ int __kprobes arch_trampoline_kprobe(struct kprobe *p)
int __init arch_init_kprobes(void)
{
register_kernel_break_hook(&kprobes_break_hook);
register_kernel_step_hook(&kprobes_step_hook);
register_kernel_break_hook(&kprobes_break_ss_hook);

return 0;
}

0 comments on commit 7ee31a3

Please sign in to comment.