Re: in stable 4.4.131 kprobes seems to be broken

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

[Adding the stable list]

On Mon, Jul 09, 2018 at 11:53:37PM -0700, Victor Kamensky wrote:
> Hi Masami,
> 
> I am not sure maybe you are aware about this issue already, but
> I am writing in case if you are not.
> 
> After move to 4.4.131 stable very basic SystemTap script that involves
> kprobes stopped working - it crashes kernel.
> 
> The SystemTap script like this:
> 
> probe kernel.function("meminfo_proc_show") {
> 	println("meminfo_proc_show called")
> }
> 
> which effectively will translate into kprobe entry in meminfo_proc_show
> function, crashes like this.
> 
> BUG: unable to handle kernel paging request at ffffffffc063f002
> IP: [<ffffffffa3c3558b>] resume_execution+0xe5/0x151
> PGD 24613067 ca
> PUD 24615067 PMD 1523e9067 PTE 3370d061
> Oops: 0003 [#1] SMP Modules linked in: <snip>
> CPU: 3 PID: 20078 Comm: <snip> Tainted: G           O    4.4.131 #1
> task: ffff880031ead780 ti: ffff880032238000 task.ti: ffff880032238000
> t /RIP: 0010:[<ffffffffa3c3558b>]  [<ffffffffa3c3558b>] resume_execution+0xe5/0x151
> RSP: 0018:ffff88017fccaec8  EFLAGS: 00010006
> RAX: ffffffffa3dad154 RBX: ffffffffa3dad152 RCX: 000000003f9c0ff9
> RDX: ffffffffc063f002 RSI: ffff88017fccaf58 RDI: 0000000000000041
> RBP: ffff8801549f83c0 R08: 0000000000000001 R09: 0000000000000001
> R10: ffff88003223bdd8 R11: 0000000000000246 R12: ffff88003223bdd8
> R13: ffff88017fccaf58 R14: ffffffffc063f000 R15: ffff88017fccee40
> FS:  00007f9f34520240(0000) GS:ffff88017fcc0000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffffc063f002 CR3: 0000000152b46000 CR4: 0000000000100670
> Stack:
>  ffff88017fccee40 ffff88017fccaf58 ffff8801549f83c0 0000000000000000
>  ffff880033cde500 0000000000000001 ffffffffa3c35633 ffff88017fccaf58
>  ffff880031ead780 0000000000000000 ffffffffa3c04e9d 0000000000004000
> Call Trace:
>  <#DB>
>  [<ffffffffa3c35633>] ? kprobe_debug_handler+0x3c/0xc8
>  [<ffffffffa3c04e9d>] ? do_debug+0x7a/0x182
>  [<ffffffffa4163afe>] ? debug+0x3e/0x70
>  [<ffffffffa3dad14d>] ? meminfo_proc_open+0x1c/0x1c
>  [<ffffffffa3d73111>] ? seq_buf_alloc+0x23/0x3c
>  <<EOE>>
>  [<ffffffffa3d735a8>] ? seq_read+0x1a6/0x40d
>  [<ffffffffa3da5700>] ? proc_reg_read+0x47/0x65
>  [<ffffffffa3da56b9>] ? proc_reg_write+0x65/0x65
>  [<ffffffffa3d55ebf>] ? __vfs_read+0x34/0xea
>  [<ffffffffa3d564f9>] ? rw_verify_area+0x7a/0xc9
>  [<ffffffffa3d565dc>] ? vfs_read+0x94/0xf9
>  [<ffffffffa3d570a4>] ? SyS_read+0x61/0xb4
>  [<ffffffffa4161b8a>] ? entry_SYSCALL_64_fastpath+0x1e/0x8e
> Code: 7d 70 00 75 3e 49 8b 95 80 00 00 00 49 39 d6 73 2b 48 89 d0 4c 29 f0
> 48 8d 48 05 48 83 f9 0e 77 1b b9 fb ff ff ff 48 01 d8 29 d1 <c6> 02 e9 01 c8
> 89 42 01 c7 45 70 01 00 00 00 eb 07 c7 45 70 ff RIP  [<ffffffffa3c3558b>]
> resume_execution+0xe5/0x151
>  RSP <ffff88017fccaec8>
> CR2: ffffffffc063f002
> 
> Decoding crash:
> 
> (gdb) p &resume_execution
> $1 = (void (*)(struct kprobe *, struct pt_regs *,
>     struct kprobe_ctlblk *)) 0xffffffff810354a6 <resume_execution>
> (gdb) b *(0xffffffff810354a6 + 0xe5)
> Breakpoint 1 at 0xffffffff8103558b: file <snip>/kernel-source/arch/x86/kernel/kprobes/core.c, line 126.
> 
> corresponds to:
> 
> static nokprobe_inline void
> __synthesize_relative_insn(void *from, void *to, u8 op)
> {
>         struct __arch_relative_insn {
>                 u8 op;
>                 s32 raddr;
>         } __packed *insn;
> 
>         insn = (struct __arch_relative_insn *)from;
>         insn->raddr = (s32)((long)(to) - ((long)(from) + 5));
>         insn->op = op; <----------------------------------------
> }
> 
> 
> (gdb) p /x &kprobe_debug_handler
> $1 = 0xffffffff810355f7
> (gdb) b *(0xffffffff810355f7 + 0x3c)
> Breakpoint 1 at 0xffffffff81035633: file <snip>/kernel-source/arch/x86/kernel/kprobes/core.c, line 932.
> 
> corresponds to
> 
> static void resume_execution(struct kprobe *p, struct pt_regs *regs,
>                              struct kprobe_ctlblk *kcb)
> {
> <snip>
>         if (p->ainsn.boostable == 0) {
>                 if ((regs->ip > copy_ip) &&
>                     (regs->ip - copy_ip) + 5 < MAX_INSN_SIZE) {
>                         /*
>                          * These instructions can be executed directly if it
>                          * jumps back to correct address.
>                          */
>                         synthesize_reljump((void *)regs->ip,
>                                 (void *)orig_ip + (regs->ip - copy_ip)); <-------
>                         p->ainsn.boostable = 1;
>                 } else {
>                         p->ainsn.boostable = -1;
>                 }
>         }
> 
> Further digging points to the following couple commits that 4.4.y stable
> picked up:
> 
> 1dd26ec7 kprobes/x86: Fix to set RWX bits correctly before releasing trampoline
> 76bee4 kprobes/x86: Set kprobes pages read-only
> 
> Effectively the way I read: the problem is that linux-4.4.y branch is
> missing your following commit
> 
> commit 804dec5bda9b4fcdab5f67fe61db4a0498af5221
> Author: Masami Hiramatsu <mhiramat@kernel.org>
> Date:   Wed Mar 29 14:00:25 2017 +0900
> 
>     kprobes/x86: Do not modify singlestep buffer while resuming
> 
>     Do not modify singlestep execution buffer (kprobe.ainsn.insn)
>     while resuming from single-stepping, instead, modifies
>     the buffer to add a jump back instruction at preparing
>     buffer.
> 
>     Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
>     Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> 
> and as result synthesize_reljump function call still exists from
> resume_execution function but it does not have proper page permission
> manipulation code around it, so making kprobes pages as
> read-only has averse effect: I.e when called from resume_execution
> function it hits read-only page and crashes.
> 
> Please advise how to proceed besides obvious backing out above mentioned
> two commits.

So if you apply the above commit, all works properly for you?

thanks,

greg k-h