[PATCH -tip V2] [BUGFIX] kprobes/x86: Do not jump-optimize kprobes on irq entry code

[PATCH -tip V2] [BUGFIX] kprobes/x86: Do not jump-optimize kprobes on irq entry code

[Masami Hiramatsu]

Since the kernel segment registers are not prepared at the
entry of irq-entry code, if a kprobe on such code is
jump-optimized, accessing per-cpu variables may cause
kernel panic.
However, if the kprobe is not optimized, it kicks int3
exception and set segment registers correctly.

This checks probe-address and if it is in irq-entry code,
it prohibits optimizing such kprobes. This means we can
continuously probing such interrupt handlers by kprobes
but it is not optimized anymore.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reported-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Tested-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---
 Changes in V2:
  - Make changes in kprobe/opt.c local, not involving unwind,
    since it requires CONFIG_FRAME_POINTER=y (Thanks Mathieu!)
---
 arch/x86/kernel/kprobes/opt.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 69ea0bc..c26e7f9 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -29,6 +29,7 @@
 #include <linux/kallsyms.h>
 #include <linux/ftrace.h>
 #include <linux/frame.h>
+#include <linux/interrupt.h>
 
 #include <asm/text-patching.h>
 #include <asm/cacheflush.h>
@@ -251,10 +252,12 @@ static int can_optimize(unsigned long paddr)
 
 	/*
 	 * Do not optimize in the entry code due to the unstable
-	 * stack handling.
+	 * stack handling and registers setup.
 	 */
-	if ((paddr >= (unsigned long)__entry_text_start) &&
-	    (paddr <  (unsigned long)__entry_text_end))
+	if (((paddr >= (unsigned long)__entry_text_start) &&
+	     (paddr <  (unsigned long)__entry_text_end)) ||
+	    ((paddr >= (unsigned long)__irqentry_text_start) &&
+	     (paddr <  (unsigned long)__irqentry_text_end)))
 		return 0;
 
 	/* Check there is enough space for a relative jump. */

Re: [PATCH -tip V2] [BUGFIX] kprobes/x86: Do not jump-optimize kprobes on irq entry code

[Ingo Molnar]

* Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Since the kernel segment registers are not prepared at the
> entry of irq-entry code, if a kprobe on such code is
> jump-optimized, accessing per-cpu variables may cause
> kernel panic.
> However, if the kprobe is not optimized, it kicks int3
> exception and set segment registers correctly.
> 
> This checks probe-address and if it is in irq-entry code,
> it prohibits optimizing such kprobes. This means we can
> continuously probing such interrupt handlers by kprobes
> but it is not optimized anymore.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Reported-by: Francis Deslauriers <francis.deslauriers@efficios.com>
> Tested-by: Francis Deslauriers <francis.deslauriers@efficios.com>
> ---
>  Changes in V2:
>   - Make changes in kprobe/opt.c local, not involving unwind,
>     since it requires CONFIG_FRAME_POINTER=y (Thanks Mathieu!)

This patch doesn't even build on x86-64 defconfig ...

arch/x86/kernel/kprobes/opt.c: In function ‘can_optimize’:
arch/x86/kernel/kprobes/opt.c:259:32: error: ‘__irqentry_text_start’ undeclared 
(first use in this function)
      ((paddr >= (unsigned long)__irqentry_text_start) &&
                                ^~~~~~~~~~~~~~~~~~~~~

Thanks,

	Ingo

Re: [PATCH -tip V2] [BUGFIX] kprobes/x86: Do not jump-optimize kprobes on irq entry code

[Masami Hiramatsu]

On Tue, 25 Jul 2017 12:40:05 +0200
Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > Since the kernel segment registers are not prepared at the
> > entry of irq-entry code, if a kprobe on such code is
> > jump-optimized, accessing per-cpu variables may cause
> > kernel panic.
> > However, if the kprobe is not optimized, it kicks int3
> > exception and set segment registers correctly.
> > 
> > This checks probe-address and if it is in irq-entry code,
> > it prohibits optimizing such kprobes. This means we can
> > continuously probing such interrupt handlers by kprobes
> > but it is not optimized anymore.
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > Reported-by: Francis Deslauriers <francis.deslauriers@efficios.com>
> > Tested-by: Francis Deslauriers <francis.deslauriers@efficios.com>
> > ---
> >  Changes in V2:
> >   - Make changes in kprobe/opt.c local, not involving unwind,
> >     since it requires CONFIG_FRAME_POINTER=y (Thanks Mathieu!)
> 
> This patch doesn't even build on x86-64 defconfig ...
> 
> arch/x86/kernel/kprobes/opt.c: In function ‘can_optimize’:
> arch/x86/kernel/kprobes/opt.c:259:32: error: ‘__irqentry_text_start’ undeclared 
> (first use in this function)
>       ((paddr >= (unsigned long)__irqentry_text_start) &&
>                                 ^~~~~~~~~~~~~~~~~~~~~

Oops, yes, hmm, I missed below ifdef...

#if defined(CONFIG_FUNCTION_GRAPH_TRACER) || defined(CONFIG_KASAN)

Or, I think we can define those as NULL in interrupt.h.

Thanks,

> 
> Thanks,
> 
> 	Ingo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH -tip V2] [BUGFIX] kprobes/x86: Do not jump-optimize kprobes on irq entry code

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3630 bytes --]

Hi Masami,

[auto build test ERROR on tip/auto-latest]
[also build test ERROR on v4.13-rc2 next-20170725]
[cannot apply to tip/x86/core]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Masami-Hiramatsu/kprobes-x86-Do-not-jump-optimize-kprobes-on-irq-entry-code/20170725-231634
config: x86_64-randconfig-x018-201730 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   arch/x86//kernel/kprobes/opt.c: In function 'can_optimize':
>> arch/x86//kernel/kprobes/opt.c:259:32: error: '__irqentry_text_start' undeclared (first use in this function)
         ((paddr >= (unsigned long)__irqentry_text_start) &&
                                   ^~~~~~~~~~~~~~~~~~~~~
   arch/x86//kernel/kprobes/opt.c:259:32: note: each undeclared identifier is reported only once for each function it appears in
>> arch/x86//kernel/kprobes/opt.c:260:32: error: '__irqentry_text_end' undeclared (first use in this function)
          (paddr <  (unsigned long)__irqentry_text_end)))
                                   ^~~~~~~~~~~~~~~~~~~

vim +/__irqentry_text_start +259 arch/x86//kernel/kprobes/opt.c

   241	
   242	/* Decode whole function to ensure any instructions don't jump into target */
   243	static int can_optimize(unsigned long paddr)
   244	{
   245		unsigned long addr, size = 0, offset = 0;
   246		struct insn insn;
   247		kprobe_opcode_t buf[MAX_INSN_SIZE];
   248	
   249		/* Lookup symbol including addr */
   250		if (!kallsyms_lookup_size_offset(paddr, &size, &offset))
   251			return 0;
   252	
   253		/*
   254		 * Do not optimize in the entry code due to the unstable
   255		 * stack handling and registers setup.
   256		 */
   257		if (((paddr >= (unsigned long)__entry_text_start) &&
   258		     (paddr <  (unsigned long)__entry_text_end)) ||
 > 259		    ((paddr >= (unsigned long)__irqentry_text_start) &&
 > 260		     (paddr <  (unsigned long)__irqentry_text_end)))
   261			return 0;
   262	
   263		/* Check there is enough space for a relative jump. */
   264		if (size - offset < RELATIVEJUMP_SIZE)
   265			return 0;
   266	
   267		/* Decode instructions */
   268		addr = paddr - offset;
   269		while (addr < paddr - offset + size) { /* Decode until function end */
   270			unsigned long recovered_insn;
   271			if (search_exception_tables(addr))
   272				/*
   273				 * Since some fixup code will jumps into this function,
   274				 * we can't optimize kprobe in this function.
   275				 */
   276				return 0;
   277			recovered_insn = recover_probed_instruction(buf, addr);
   278			if (!recovered_insn)
   279				return 0;
   280			kernel_insn_init(&insn, (void *)recovered_insn, MAX_INSN_SIZE);
   281			insn_get_length(&insn);
   282			/* Another subsystem puts a breakpoint */
   283			if (insn.opcode.bytes[0] == BREAKPOINT_INSTRUCTION)
   284				return 0;
   285			/* Recover address */
   286			insn.kaddr = (void *)addr;
   287			insn.next_byte = (void *)(addr + insn.length);
   288			/* Check any instructions don't jump into target */
   289			if (insn_is_indirect_jump(&insn) ||
   290			    insn_jump_into_range(&insn, paddr + INT3_SIZE,
   291						 RELATIVE_ADDR_SIZE))
   292				return 0;
   293			addr += insn.length;
   294		}
   295	
   296		return 1;
   297	}
   298	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24044 bytes --]