On Fri, 11 Dec 2020 16:04:17 +0900
Masami Hiramatsu <[email protected]> wrote:
> Fix optprobe to detect padding int3 correctly.
>
> Since commit 7705dc855797 ("x86/vmlinux: Use INT3 instead of NOP
> for linker fill bytes") changed the padding bytes between functions
> from nop to int3, when optprobe decodes a target function it finds
> int3 and gives up the jump optimization.
>
> Instead of giving up any int3 detection, this checks whether the
> rest of bytes to the end of the function are int3 or not. If all
> of those are int3, those come from the linker. In that case,
> optprobe continues jump optimization.
>
> Fixes: 7705dc855797 ("x86/vmlinux: Use INT3 instead of NOP for linker fill
> bytes")
> Cc: [email protected]
> Reported-by: Adam Zabrocki <[email protected]>
> Signed-off-by: Masami Hiramatsu <[email protected]>
Reviewed-by: Steven Rostedt (VMware) <[email protected]>
This should probably go in via tip.
-- Steve
> ---
> arch/x86/kernel/kprobes/opt.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
> index 041f0b50bc27..b5cf39f1a855 100644
> --- a/arch/x86/kernel/kprobes/opt.c
> +++ b/arch/x86/kernel/kprobes/opt.c
> @@ -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)
> {
> @@ -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);
