linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/kprobes: Fix optprobe to detect padding int3 correctly
@ 2020-12-11  7:04 Masami Hiramatsu
  2020-12-11 14:49 ` Steven Rostedt
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Masami Hiramatsu @ 2020-12-11  7:04 UTC (permalink / raw)
  To: Ingo Molnar, Steven Rostedt
  Cc: Adam Zabrocki, Masami Hiramatsu, Thomas Gleixner, Ingo Molnar,
	Kees Cook, Borislav Petkov, x86, H . Peter Anvin, linux-kernel,
	Naveen N . Rao, Anil S Keshavamurthy, David S . Miller,
	Solar Designer

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: stable@vger.kernel.org
Reported-by: Adam Zabrocki <pi3@pi3.com.pl>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 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);


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/kprobes: Fix optprobe to detect padding int3 correctly
  2020-12-11  7:04 [PATCH] x86/kprobes: Fix optprobe to detect padding int3 correctly Masami Hiramatsu
@ 2020-12-11 14:49 ` Steven Rostedt
  2020-12-11 19:39 ` Kees Cook
  2020-12-12 14:38 ` [tip: x86/urgent] x86/kprobes: Fix optprobe to detect INT3 padding correctly tip-bot2 for Masami Hiramatsu
  2 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2020-12-11 14:49 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Adam Zabrocki, Thomas Gleixner, Ingo Molnar,
	Kees Cook, Borislav Petkov, x86, H . Peter Anvin, linux-kernel,
	Naveen N . Rao, Anil S Keshavamurthy, David S . Miller,
	Solar Designer

On Fri, 11 Dec 2020 16:04:17 +0900
Masami Hiramatsu <mhiramat@kernel.org> 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: stable@vger.kernel.org
> Reported-by: Adam Zabrocki <pi3@pi3.com.pl>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

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);


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/kprobes: Fix optprobe to detect padding int3 correctly
  2020-12-11  7:04 [PATCH] x86/kprobes: Fix optprobe to detect padding int3 correctly Masami Hiramatsu
  2020-12-11 14:49 ` Steven Rostedt
@ 2020-12-11 19:39 ` Kees Cook
  2020-12-11 20:04   ` Adam Zabrocki
  2020-12-12 14:38 ` [tip: x86/urgent] x86/kprobes: Fix optprobe to detect INT3 padding correctly tip-bot2 for Masami Hiramatsu
  2 siblings, 1 reply; 5+ messages in thread
From: Kees Cook @ 2020-12-11 19:39 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Steven Rostedt, Adam Zabrocki, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, H . Peter Anvin, linux-kernel,
	Naveen N . Rao, Anil S Keshavamurthy, David S . Miller,
	Solar Designer

On Fri, Dec 11, 2020 at 04:04:17PM +0900, Masami Hiramatsu 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: stable@vger.kernel.org
> Reported-by: Adam Zabrocki <pi3@pi3.com.pl>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] x86/kprobes: Fix optprobe to detect padding int3 correctly
  2020-12-11 19:39 ` Kees Cook
@ 2020-12-11 20:04   ` Adam Zabrocki
  0 siblings, 0 replies; 5+ messages in thread
From: Adam Zabrocki @ 2020-12-11 20:04 UTC (permalink / raw)
  To: Kees Cook
  Cc: Masami Hiramatsu, Ingo Molnar, Steven Rostedt, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, x86, H . Peter Anvin, linux-kernel,
	Naveen N . Rao, Anil S Keshavamurthy, David S . Miller,
	Solar Designer

Hi,

I've applied this patch on the top of kernel 5.9.7 and verified it works 
fine:

ffffffff813050e0 <ftrace_enable_sysctl>:
...
...
ffffffff813052f1:       e9 fe fe ff ff          jmpq   ffffffff813051f4 <ftrace_enable_sysctl+0x114>
ffffffff813052f6:       cc                      int3
ffffffff813052f7:       cc                      int3
ffffffff813052f8:       cc                      int3
ffffffff813052f9:       cc                      int3
ffffffff813052fa:       cc                      int3
ffffffff813052fb:       cc                      int3
ffffffff813052fc:       cc                      int3
ffffffff813052fd:       cc                      int3
ffffffff813052fe:       cc                      int3
ffffffff813052ff:       cc                      int3

When I install KRETPROBE on this function, it is correctly optimized:

root@pi3:~/off-debug/git/lkrg# cat /sys/kernel/debug/kprobes/list|grep ftrace_enable_sysctl
ffffffff813050e0  r  ftrace_enable_sysctl+0x0    [OPTIMIZED]
root@pi3:~/off-debug/git/lkrg# 

gef➤  x/2i ftrace_enable_sysctl
   0xffffffff813050e0 <ftrace_enable_sysctl>:   jmp    0xffffffffc062807a
   0xffffffff813050e5 <ftrace_enable_sysctl+5>: push   r14

Thanks,
Adam

On Fri, Dec 11, 2020 at 11:39:15AM -0800, Kees Cook wrote:
> On Fri, Dec 11, 2020 at 04:04:17PM +0900, Masami Hiramatsu 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: stable@vger.kernel.org
> > Reported-by: Adam Zabrocki <pi3@pi3.com.pl>
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> 
> Reviewed-by: Kees Cook <keescook@chromium.org>
> 
> -- 
> Kees Cook

-- 
pi3 (pi3ki31ny) - pi3 (at) itsec pl
http://pi3.com.pl


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tip: x86/urgent] x86/kprobes: Fix optprobe to detect INT3 padding correctly
  2020-12-11  7:04 [PATCH] x86/kprobes: Fix optprobe to detect padding int3 correctly Masami Hiramatsu
  2020-12-11 14:49 ` Steven Rostedt
  2020-12-11 19:39 ` Kees Cook
@ 2020-12-12 14:38 ` tip-bot2 for Masami Hiramatsu
  2 siblings, 0 replies; 5+ messages in thread
From: tip-bot2 for Masami Hiramatsu @ 2020-12-12 14:38 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Adam Zabrocki, Masami Hiramatsu, Borislav Petkov,
	Steven Rostedt (VMware),
	Kees Cook, stable, x86, linux-kernel

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     0d07c0ec4381f630c801539c79ad8dcc627f6e4a
Gitweb:        https://git.kernel.org/tip/0d07c0ec4381f630c801539c79ad8dcc627f6e4a
Author:        Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate:    Fri, 11 Dec 2020 16:04:17 +09:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Sat, 12 Dec 2020 15:25:17 +01:00

x86/kprobes: Fix optprobe to detect INT3 padding correctly

Commit

  7705dc855797 ("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: 7705dc855797 ("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
---
 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 041f0b5..08eb230 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);

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-12-12 14:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-11  7:04 [PATCH] x86/kprobes: Fix optprobe to detect padding int3 correctly Masami Hiramatsu
2020-12-11 14:49 ` Steven Rostedt
2020-12-11 19:39 ` Kees Cook
2020-12-11 20:04   ` Adam Zabrocki
2020-12-12 14:38 ` [tip: x86/urgent] x86/kprobes: Fix optprobe to detect INT3 padding correctly tip-bot2 for Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).