Skip to content

Commit

Permalink
x86/kprobes: Fix optprobe to detect INT3 padding correctly
Browse files Browse the repository at this point in the history
Commit

  7705dc8 ("x86/vmlinux: Use INT3 instead of NOP for linker fill bytes")

changed the padding bytes between functions from NOP to INT3. However,
when optprobe decodes a target function it finds INT3 and gives up the
jump optimization.

Instead of giving up any INT3 detection, check whether the rest of the
bytes to the end of the function are INT3. If all of them are INT3,
those come from the linker. In that case, continue the optprobe jump
optimization.

 [ bp: Massage commit message. ]

Fixes: 7705dc8 ("x86/vmlinux: Use INT3 instead of NOP for linker fill bytes")
Reported-by: Adam Zabrocki <pi3@pi3.com.pl>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/160767025681.3880685.16021570341428835411.stgit@devnote2
  • Loading branch information
Masami Hiramatsu authored and Borislav Petkov committed Dec 12, 2020
1 parent 190113b commit 0d07c0e
Showing 1 changed file with 20 additions and 2 deletions.
22 changes: 20 additions & 2 deletions arch/x86/kernel/kprobes/opt.c
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,19 @@ static int insn_is_indirect_jump(struct insn *insn)
return ret;
}

static bool is_padding_int3(unsigned long addr, unsigned long eaddr)
{
unsigned char ops;

for (; addr < eaddr; addr++) {
if (get_kernel_nofault(ops, (void *)addr) < 0 ||
ops != INT3_INSN_OPCODE)
return false;
}

return true;
}

/* Decode whole function to ensure any instructions don't jump into target */
static int can_optimize(unsigned long paddr)
{
Expand Down Expand Up @@ -310,9 +323,14 @@ static int can_optimize(unsigned long paddr)
return 0;
kernel_insn_init(&insn, (void *)recovered_insn, MAX_INSN_SIZE);
insn_get_length(&insn);
/* Another subsystem puts a breakpoint */
/*
* In the case of detecting unknown breakpoint, this could be
* a padding INT3 between functions. Let's check that all the
* rest of the bytes are also INT3.
*/
if (insn.opcode.bytes[0] == INT3_INSN_OPCODE)
return 0;
return is_padding_int3(addr, paddr - offset + size) ? 1 : 0;

/* Recover address */
insn.kaddr = (void *)addr;
insn.next_byte = (void *)(addr + insn.length);
Expand Down

0 comments on commit 0d07c0e

Please sign in to comment.