All of lore.kernel.org
 help / color / mirror / Atom feed
* WARNING in arch_uprobe_analyze_insn
@ 2018-05-14 14:24 syzbot
  2018-05-15 13:36 ` Oleg Nesterov
  2018-05-18 16:27 ` [PATCH] uprobes/x86: remove the wrong WARN_ON() in uprobe_init_insn() Oleg Nesterov
  0 siblings, 2 replies; 6+ messages in thread
From: syzbot @ 2018-05-14 14:24 UTC (permalink / raw)
  To: bp, hpa, linux-kernel, mingo, oleg, ricardo.neri-calderon,
	syzkaller-bugs, tglx, x86, yhs

Hello,

syzbot found the following crash on:

HEAD commit:    66e1c94db3cd Merge branch 'x86-pti-for-linus' of git://git..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11817107800000
kernel config:  https://syzkaller.appspot.com/x/.config?x=fcce42b221691ff9
dashboard link: https://syzkaller.appspot.com/bug?extid=30d675e3ca03c1c351e7
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=143d5417800000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=11178e97800000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+30d675e3ca03c1c351e7@syzkaller.appspotmail.com

random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
WARNING: CPU: 0 PID: 4511 at arch/x86/kernel/uprobes.c:296 insn_complete  
arch/x86/include/asm/insn.h:147 [inline]
WARNING: CPU: 0 PID: 4511 at arch/x86/kernel/uprobes.c:296 uprobe_init_insn  
arch/x86/kernel/uprobes.c:296 [inline]
WARNING: CPU: 0 PID: 4511 at arch/x86/kernel/uprobes.c:296  
arch_uprobe_analyze_insn+0x13d/0x15f0 arch/x86/kernel/uprobes.c:861
Kernel panic - not syncing: panic_on_warn set ...

CPU: 0 PID: 4511 Comm: syz-executor347 Not tainted 4.17.0-rc4+ #46
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
  panic+0x22f/0x4de kernel/panic.c:184
  __warn.cold.8+0x163/0x1b3 kernel/panic.c:536
  report_bug+0x252/0x2d0 lib/bug.c:186
  fixup_bug arch/x86/kernel/traps.c:178 [inline]
  do_error_trap+0x1de/0x490 arch/x86/kernel/traps.c:296
  do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
RIP: 0010:insn_complete arch/x86/include/asm/insn.h:147 [inline]
RIP: 0010:uprobe_init_insn arch/x86/kernel/uprobes.c:296 [inline]
RIP: 0010:arch_uprobe_analyze_insn+0x13d/0x15f0  
arch/x86/kernel/uprobes.c:861
RSP: 0018:ffff8801cf277510 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff8801cf277560 RCX: ffffffff876ccf2a
RDX: 0000000000000004 RSI: ffffffff876ce6ab RDI: ffff8801cf27759c
RBP: ffff8801cf277628 R08: ffff8801d8c0a5c0 R09: ffff8801cf277560
R10: ffff8801d7ad81d0 R11: ffff8801cf2775af R12: 0000000000000000
R13: dffffc0000000000 R14: ffff8801d7ad8080 R15: ffff8801cf277600
  prepare_uprobe kernel/events/uprobes.c:610 [inline]
  install_breakpoint.isra.21+0x710/0x830 kernel/events/uprobes.c:657
  uprobe_mmap+0x6a0/0xcf0 kernel/events/uprobes.c:1089
  mmap_region+0x5c8/0x1870 mm/mmap.c:1807
  do_mmap+0xde2/0x1360 mm/mmap.c:1535
  do_mmap_pgoff include/linux/mm.h:2237 [inline]
  vm_mmap_pgoff+0x1fb/0x2a0 mm/util.c:357
  ksys_mmap_pgoff+0x4c9/0x640 mm/mmap.c:1585
  __do_sys_mmap arch/x86/kernel/sys_x86_64.c:100 [inline]
  __se_sys_mmap arch/x86/kernel/sys_x86_64.c:91 [inline]
  __x64_sys_mmap+0xe9/0x1b0 arch/x86/kernel/sys_x86_64.c:91
  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x43ff29
RSP: 002b:00007ffc922a4898 EFLAGS: 00000212 ORIG_RAX: 0000000000000009
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 000000000043ff29
RDX: 0000000000000000 RSI: 0000000000002000 RDI: 000000002000c000
RBP: 00000000006ca018 R08: 0000000000000003 R09: 0000000000000000
R10: 0000000000000012 R11: 0000000000000212 R12: 0000000000401850
R13: 00000000004018e0 R14: 0000000000000000 R15: 0000000000000000
Dumping ftrace buffer:
    (ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

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

* Re: WARNING in arch_uprobe_analyze_insn
  2018-05-14 14:24 WARNING in arch_uprobe_analyze_insn syzbot
@ 2018-05-15 13:36 ` Oleg Nesterov
  2018-05-15 14:48   ` Masami Hiramatsu
  2018-05-18 16:27 ` [PATCH] uprobes/x86: remove the wrong WARN_ON() in uprobe_init_insn() Oleg Nesterov
  1 sibling, 1 reply; 6+ messages in thread
From: Oleg Nesterov @ 2018-05-15 13:36 UTC (permalink / raw)
  To: syzbot, Masami Hiramatsu
  Cc: bp, hpa, linux-kernel, mingo, ricardo.neri-calderon,
	syzkaller-bugs, tglx, x86, yhs

On 05/14, syzbot wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    66e1c94db3cd Merge branch 'x86-pti-for-linus' of git://git..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=11817107800000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=fcce42b221691ff9
> dashboard link: https://syzkaller.appspot.com/bug?extid=30d675e3ca03c1c351e7
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=143d5417800000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=11178e97800000

doesn't work for me...

I guess syscall(__NR_perf_event_open) should trigger perf_uprobe_init() but
the hard-coded attr.type doesn't match perf_uprobe->type on my machine...


Nevermind, it seems that the code is indeed wrong, uprobe_init_insn() does

	insn_init(insn, auprobe->insn, sizeof(auprobe->insn), x86_64);
	/* has the side-effect of processing the entire instruction */
	insn_get_length(insn);
	if (WARN_ON_ONCE(!insn_complete(insn)))
		return -ENOEXEC;

Masami, could your help?

What if insn can't be decoded, say, because it is not valid? Can't
insn_complete() return F in this case?

I think it can... Say, insn_get_immediate() doesn't set immediate.got in
err_out path.

Unfortunately, insn_get_length() returns void, and I do not see any
insn-was-decoded-correctly helper. Perhaps we should simply remove
this WARN_ON() ?

Alternatively, If am right we can move this check down after the "good_insns"
checks, but this doesn't look very clean to me.


> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+30d675e3ca03c1c351e7@syzkaller.appspotmail.com
> 
> random: sshd: uninitialized urandom read (32 bytes read)
> random: sshd: uninitialized urandom read (32 bytes read)
> random: sshd: uninitialized urandom read (32 bytes read)
> random: sshd: uninitialized urandom read (32 bytes read)
> random: sshd: uninitialized urandom read (32 bytes read)
> WARNING: CPU: 0 PID: 4511 at arch/x86/kernel/uprobes.c:296 insn_complete
> arch/x86/include/asm/insn.h:147 [inline]
> WARNING: CPU: 0 PID: 4511 at arch/x86/kernel/uprobes.c:296 uprobe_init_insn
> arch/x86/kernel/uprobes.c:296 [inline]
> WARNING: CPU: 0 PID: 4511 at arch/x86/kernel/uprobes.c:296
> arch_uprobe_analyze_insn+0x13d/0x15f0 arch/x86/kernel/uprobes.c:861
> Kernel panic - not syncing: panic_on_warn set ...
> 
> CPU: 0 PID: 4511 Comm: syz-executor347 Not tainted 4.17.0-rc4+ #46
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
>  panic+0x22f/0x4de kernel/panic.c:184
>  __warn.cold.8+0x163/0x1b3 kernel/panic.c:536
>  report_bug+0x252/0x2d0 lib/bug.c:186
>  fixup_bug arch/x86/kernel/traps.c:178 [inline]
>  do_error_trap+0x1de/0x490 arch/x86/kernel/traps.c:296
>  do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
>  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
> RIP: 0010:insn_complete arch/x86/include/asm/insn.h:147 [inline]
> RIP: 0010:uprobe_init_insn arch/x86/kernel/uprobes.c:296 [inline]
> RIP: 0010:arch_uprobe_analyze_insn+0x13d/0x15f0
> arch/x86/kernel/uprobes.c:861
> RSP: 0018:ffff8801cf277510 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffff8801cf277560 RCX: ffffffff876ccf2a
> RDX: 0000000000000004 RSI: ffffffff876ce6ab RDI: ffff8801cf27759c
> RBP: ffff8801cf277628 R08: ffff8801d8c0a5c0 R09: ffff8801cf277560
> R10: ffff8801d7ad81d0 R11: ffff8801cf2775af R12: 0000000000000000
> R13: dffffc0000000000 R14: ffff8801d7ad8080 R15: ffff8801cf277600
>  prepare_uprobe kernel/events/uprobes.c:610 [inline]
>  install_breakpoint.isra.21+0x710/0x830 kernel/events/uprobes.c:657
>  uprobe_mmap+0x6a0/0xcf0 kernel/events/uprobes.c:1089
>  mmap_region+0x5c8/0x1870 mm/mmap.c:1807
>  do_mmap+0xde2/0x1360 mm/mmap.c:1535
>  do_mmap_pgoff include/linux/mm.h:2237 [inline]
>  vm_mmap_pgoff+0x1fb/0x2a0 mm/util.c:357
>  ksys_mmap_pgoff+0x4c9/0x640 mm/mmap.c:1585
>  __do_sys_mmap arch/x86/kernel/sys_x86_64.c:100 [inline]
>  __se_sys_mmap arch/x86/kernel/sys_x86_64.c:91 [inline]
>  __x64_sys_mmap+0xe9/0x1b0 arch/x86/kernel/sys_x86_64.c:91
>  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x43ff29
> RSP: 002b:00007ffc922a4898 EFLAGS: 00000212 ORIG_RAX: 0000000000000009
> RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 000000000043ff29
> RDX: 0000000000000000 RSI: 0000000000002000 RDI: 000000002000c000
> RBP: 00000000006ca018 R08: 0000000000000003 R09: 0000000000000000
> R10: 0000000000000012 R11: 0000000000000212 R12: 0000000000401850
> R13: 00000000004018e0 R14: 0000000000000000 R15: 0000000000000000
> Dumping ftrace buffer:
>    (ftrace buffer empty)
> Kernel Offset: disabled
> Rebooting in 86400 seconds..
> 
> 
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
> 
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches

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

* Re: WARNING in arch_uprobe_analyze_insn
  2018-05-15 13:36 ` Oleg Nesterov
@ 2018-05-15 14:48   ` Masami Hiramatsu
  2018-05-15 16:18     ` Oleg Nesterov
  0 siblings, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2018-05-15 14:48 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: syzbot, bp, hpa, linux-kernel, mingo, ricardo.neri-calderon,
	syzkaller-bugs, tglx, x86, yhs

On Tue, 15 May 2018 15:36:30 +0200
Oleg Nesterov <oleg@redhat.com> wrote:

> On 05/14, syzbot wrote:
> >
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit:    66e1c94db3cd Merge branch 'x86-pti-for-linus' of git://git..
> > git tree:       upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=11817107800000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=fcce42b221691ff9
> > dashboard link: https://syzkaller.appspot.com/bug?extid=30d675e3ca03c1c351e7
> > compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> > syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=143d5417800000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=11178e97800000
> 
> doesn't work for me...
> 
> I guess syscall(__NR_perf_event_open) should trigger perf_uprobe_init() but
> the hard-coded attr.type doesn't match perf_uprobe->type on my machine...
> 
> 
> Nevermind, it seems that the code is indeed wrong, uprobe_init_insn() does
> 
> 	insn_init(insn, auprobe->insn, sizeof(auprobe->insn), x86_64);
> 	/* has the side-effect of processing the entire instruction */
> 	insn_get_length(insn);
> 	if (WARN_ON_ONCE(!insn_complete(insn)))
> 		return -ENOEXEC;
> 
> Masami, could your help?
> 
> What if insn can't be decoded, say, because it is not valid? Can't
> insn_complete() return F in this case?

Yes, as far as it failes to decode.

BTW, insn_complete() may not work some errors, it is hard to find
all invalid combination of x86 insn. I think the uprobes is
fundamentary week about fuzzing code because it can not
identify the instruction boundary in user space.

> I think it can... Say, insn_get_immediate() doesn't set immediate.got in
> err_out path.
> 
> Unfortunately, insn_get_length() returns void, and I do not see any
> insn-was-decoded-correctly helper. Perhaps we should simply remove
> this WARN_ON() ?

Yes, it should just return an error, since user can miss the
probe address on user binary and we can not make sure
that is on a instruction boundary.

> Alternatively, If am right we can move this check down after the "good_insns"
> checks, but this doesn't look very clean to me.

I think it is enough if arch_uprobe_analyze_insn() returns an error.
That makes prepare_uprobe() fail and finally leads uprobe_register()
fail.

Thank you,

> 
> 
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+30d675e3ca03c1c351e7@syzkaller.appspotmail.com
> > 
> > random: sshd: uninitialized urandom read (32 bytes read)
> > random: sshd: uninitialized urandom read (32 bytes read)
> > random: sshd: uninitialized urandom read (32 bytes read)
> > random: sshd: uninitialized urandom read (32 bytes read)
> > random: sshd: uninitialized urandom read (32 bytes read)
> > WARNING: CPU: 0 PID: 4511 at arch/x86/kernel/uprobes.c:296 insn_complete
> > arch/x86/include/asm/insn.h:147 [inline]
> > WARNING: CPU: 0 PID: 4511 at arch/x86/kernel/uprobes.c:296 uprobe_init_insn
> > arch/x86/kernel/uprobes.c:296 [inline]
> > WARNING: CPU: 0 PID: 4511 at arch/x86/kernel/uprobes.c:296
> > arch_uprobe_analyze_insn+0x13d/0x15f0 arch/x86/kernel/uprobes.c:861
> > Kernel panic - not syncing: panic_on_warn set ...
> > 
> > CPU: 0 PID: 4511 Comm: syz-executor347 Not tainted 4.17.0-rc4+ #46
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > Google 01/01/2011
> > Call Trace:
> >  __dump_stack lib/dump_stack.c:77 [inline]
> >  dump_stack+0x1b9/0x294 lib/dump_stack.c:113
> >  panic+0x22f/0x4de kernel/panic.c:184
> >  __warn.cold.8+0x163/0x1b3 kernel/panic.c:536
> >  report_bug+0x252/0x2d0 lib/bug.c:186
> >  fixup_bug arch/x86/kernel/traps.c:178 [inline]
> >  do_error_trap+0x1de/0x490 arch/x86/kernel/traps.c:296
> >  do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:315
> >  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992
> > RIP: 0010:insn_complete arch/x86/include/asm/insn.h:147 [inline]
> > RIP: 0010:uprobe_init_insn arch/x86/kernel/uprobes.c:296 [inline]
> > RIP: 0010:arch_uprobe_analyze_insn+0x13d/0x15f0
> > arch/x86/kernel/uprobes.c:861
> > RSP: 0018:ffff8801cf277510 EFLAGS: 00010246
> > RAX: 0000000000000000 RBX: ffff8801cf277560 RCX: ffffffff876ccf2a
> > RDX: 0000000000000004 RSI: ffffffff876ce6ab RDI: ffff8801cf27759c
> > RBP: ffff8801cf277628 R08: ffff8801d8c0a5c0 R09: ffff8801cf277560
> > R10: ffff8801d7ad81d0 R11: ffff8801cf2775af R12: 0000000000000000
> > R13: dffffc0000000000 R14: ffff8801d7ad8080 R15: ffff8801cf277600
> >  prepare_uprobe kernel/events/uprobes.c:610 [inline]
> >  install_breakpoint.isra.21+0x710/0x830 kernel/events/uprobes.c:657
> >  uprobe_mmap+0x6a0/0xcf0 kernel/events/uprobes.c:1089
> >  mmap_region+0x5c8/0x1870 mm/mmap.c:1807
> >  do_mmap+0xde2/0x1360 mm/mmap.c:1535
> >  do_mmap_pgoff include/linux/mm.h:2237 [inline]
> >  vm_mmap_pgoff+0x1fb/0x2a0 mm/util.c:357
> >  ksys_mmap_pgoff+0x4c9/0x640 mm/mmap.c:1585
> >  __do_sys_mmap arch/x86/kernel/sys_x86_64.c:100 [inline]
> >  __se_sys_mmap arch/x86/kernel/sys_x86_64.c:91 [inline]
> >  __x64_sys_mmap+0xe9/0x1b0 arch/x86/kernel/sys_x86_64.c:91
> >  do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
> >  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > RIP: 0033:0x43ff29
> > RSP: 002b:00007ffc922a4898 EFLAGS: 00000212 ORIG_RAX: 0000000000000009
> > RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 000000000043ff29
> > RDX: 0000000000000000 RSI: 0000000000002000 RDI: 000000002000c000
> > RBP: 00000000006ca018 R08: 0000000000000003 R09: 0000000000000000
> > R10: 0000000000000012 R11: 0000000000000212 R12: 0000000000401850
> > R13: 00000000004018e0 R14: 0000000000000000 R15: 0000000000000000
> > Dumping ftrace buffer:
> >    (ftrace buffer empty)
> > Kernel Offset: disabled
> > Rebooting in 86400 seconds..
> > 
> > 
> > ---
> > This bug is generated by a bot. It may contain errors.
> > See https://goo.gl/tpsmEJ for more information about syzbot.
> > syzbot engineers can be reached at syzkaller@googlegroups.com.
> > 
> > syzbot will keep track of this bug report. See:
> > https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> > syzbot.
> > syzbot can test patches for this bug, for details see:
> > https://goo.gl/tpsmEJ#testing-patches
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

* Re: WARNING in arch_uprobe_analyze_insn
  2018-05-15 14:48   ` Masami Hiramatsu
@ 2018-05-15 16:18     ` Oleg Nesterov
  0 siblings, 0 replies; 6+ messages in thread
From: Oleg Nesterov @ 2018-05-15 16:18 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: syzbot, bp, hpa, linux-kernel, mingo, ricardo.neri-calderon,
	syzkaller-bugs, tglx, x86, yhs

On 05/15, Masami Hiramatsu wrote:
>
> On Tue, 15 May 2018 15:36:30 +0200
> Oleg Nesterov <oleg@redhat.com> wrote:
>
> > Unfortunately, insn_get_length() returns void, and I do not see any
> > insn-was-decoded-correctly helper. Perhaps we should simply remove
> > this WARN_ON() ?
>
> Yes, it should just return an error,

OK, I'll send the fix,

> since user can miss the
> probe address on user binary and we can not make sure
> that is on a instruction boundary.

Or this insn is actually invalid.

> > Alternatively, If am right we can move this check down after the "good_insns"
> > checks, but this doesn't look very clean to me.
>
> I think it is enough if arch_uprobe_analyze_insn() returns an error.
> That makes prepare_uprobe() fail and finally leads uprobe_register()
> fail.

I meant that this way we could probably keep WARN_ON(). Nevermind.

Thanks!

Oleg.

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

* [PATCH] uprobes/x86: remove the wrong WARN_ON() in uprobe_init_insn()
  2018-05-14 14:24 WARNING in arch_uprobe_analyze_insn syzbot
  2018-05-15 13:36 ` Oleg Nesterov
@ 2018-05-18 16:27 ` Oleg Nesterov
  2018-05-18 23:58   ` Masami Hiramatsu
  1 sibling, 1 reply; 6+ messages in thread
From: Oleg Nesterov @ 2018-05-18 16:27 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, syzkaller-bugs, syzbot, Masami Hiramatsu

insn_get_length() has the side-effect of processing the entire instruction
but only if it was decoded successfully, otherwise insn_complete() can fail
and in this case we need to just return an error without warning.

Reported-by: syzbot+30d675e3ca03c1c351e7@syzkaller.appspotmail.com
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 arch/x86/kernel/uprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 85c7ef2..efd7ee9 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -293,7 +293,7 @@ static int uprobe_init_insn(struct arch_uprobe *auprobe, struct insn *insn, bool
 	insn_init(insn, auprobe->insn, sizeof(auprobe->insn), x86_64);
 	/* has the side-effect of processing the entire instruction */
 	insn_get_length(insn);
-	if (WARN_ON_ONCE(!insn_complete(insn)))
+	if (!insn_complete(insn))
 		return -ENOEXEC;
 
 	if (is_prefix_bad(insn))
-- 
2.5.0

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

* Re: [PATCH] uprobes/x86: remove the wrong WARN_ON() in uprobe_init_insn()
  2018-05-18 16:27 ` [PATCH] uprobes/x86: remove the wrong WARN_ON() in uprobe_init_insn() Oleg Nesterov
@ 2018-05-18 23:58   ` Masami Hiramatsu
  0 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2018-05-18 23:58 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Ingo Molnar, linux-kernel, syzkaller-bugs, syzbot, Masami Hiramatsu

On Fri, 18 May 2018 18:27:39 +0200
Oleg Nesterov <oleg@redhat.com> wrote:

> insn_get_length() has the side-effect of processing the entire instruction
> but only if it was decoded successfully, otherwise insn_complete() can fail
> and in this case we need to just return an error without warning.
> 

This looks good to me.

Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!

> Reported-by: syzbot+30d675e3ca03c1c351e7@syzkaller.appspotmail.com
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
>  arch/x86/kernel/uprobes.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> index 85c7ef2..efd7ee9 100644
> --- a/arch/x86/kernel/uprobes.c
> +++ b/arch/x86/kernel/uprobes.c
> @@ -293,7 +293,7 @@ static int uprobe_init_insn(struct arch_uprobe *auprobe, struct insn *insn, bool
>  	insn_init(insn, auprobe->insn, sizeof(auprobe->insn), x86_64);
>  	/* has the side-effect of processing the entire instruction */
>  	insn_get_length(insn);
> -	if (WARN_ON_ONCE(!insn_complete(insn)))
> +	if (!insn_complete(insn))
>  		return -ENOEXEC;
>  
>  	if (is_prefix_bad(insn))
> -- 
> 2.5.0
> 
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

end of thread, other threads:[~2018-05-18 23:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-14 14:24 WARNING in arch_uprobe_analyze_insn syzbot
2018-05-15 13:36 ` Oleg Nesterov
2018-05-15 14:48   ` Masami Hiramatsu
2018-05-15 16:18     ` Oleg Nesterov
2018-05-18 16:27 ` [PATCH] uprobes/x86: remove the wrong WARN_ON() in uprobe_init_insn() Oleg Nesterov
2018-05-18 23:58   ` Masami Hiramatsu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.