Skip to contentSkip to conversation

WARNING in arch_uprobe_analyze_insn

PATCH In review syzbot 3 participants 6 messages May 14, 2018 – May 18, 2018

Subsystems (2)
x86 uprobes
lore
Share & tools
lore

syzbot

author syzbot
#0 lore raw

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 [email protected]

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 [email protected].

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

maintainer
#1 lore raw
Quoted 5 lines · syzbot

Hello,

syzbot found the following crash on:

Quoted 8 lines
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.

Quoted 75 lines

IMPORTANT: if you fix the bug, please add the following tag to the commit:

Reported-by [email protected]

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 [email protected].

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

maintainer
#2 lore raw
Quoted 29 lines · On Tue, 15 May 2018 15:36:30 +0200 Oleg Nesterov <[email protected]> wrote:
Quoted 5 lines · syzbot

Hello,

syzbot found the following crash on:

Quoted 8 lines
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;
Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 2 lines

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,

Quoted 78 lines
Quoted 77 lines

IMPORTANT: if you fix the bug, please add the following tag to the commit:

Reported-by [email protected]

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 [email protected].

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

Signature · 1 line
Masami Hiramatsu <[email protected]>
maintainer
#3 lore raw
Quoted 9 lines · Masami Hiramatsu
Quoted 3 lines · On Tue, 15 May 2018 15:36:30 +0200 Oleg Nesterov <[email protected]> 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,

Quoted 3 lines

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.

Quoted 6 lines
Quoted 2 lines

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.

maintainer
#4 lore raw

PATCH 1/1 uprobes/x86: remove the wrong WARN_ON() in uprobe_init_insn()

In reply to syzbot

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 [email protected]
Signed-off-by Oleg Nesterov <[email protected]>
---
 arch/x86/kernel/uprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/uprobes.c +1 -1
index 85c7ef2..efd7ee9 100644
@@ -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))
Signature · 1 line
2.5.0
maintainer Reviewed-by on v1 1/1
#5 lore raw

Re: [PATCH] uprobes/x86: remove the wrong WARN_ON() in uprobe_init_insn()

Quoted 4 lines · On Fri, 18 May 2018 18:27:39 +0200 Oleg Nesterov <[email protected]> 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 <[email protected]>

Thanks!

Quoted 23 lines
Reported-by [email protected]
Signed-off-by Oleg Nesterov <[email protected]>
---
 arch/x86/kernel/uprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/uprobes.c +1 -1
index 85c7ef2..efd7ee9 100644
@@ -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))
Signature · 1 line
2.5.0
Signature · 1 line
Masami Hiramatsu <[email protected]>