2024-03-13 02:53:39

by cheung wall

[permalink] [raw]
Subject: BUG: unable to handle kernel paging request in arch_adjust_kprobe_addr

Hello,


when using Healer to fuzz the latest Linux Kernel, the following crash

was triggered on:


HEAD commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a (tag: v6.7)

git tree: upstream

console output: https://pastebin.com/raw/iw2bFsWa

kernel config: https://pastebin.com/raw/Ta59KYzh

C reproducer: https://pastebin.com/raw/JDqeSxiK

Syzlang reproducer: https://pastebin.com/raw/Vjs199hz


If you fix this issue, please add the following tag to the commit:

Reported-by: Qiang Zhang <[email protected]>

----------------------------------------------------------


BUG: unable to handle page fault for address: ffffffff95003e80
audit: type=1400 audit(1710291918.880:7): avc: denied { open } for
pid=298 comm="syz-executor372" scontext=system_u:system_r:kernel_t:s0
tcontext=system_u:system_r:kernel_t:s0 tclass=perf_event permissive=1
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 1c4a7067 P4D 1c4a7067 PUD 1c4a8063
audit: type=1400 audit(1710291918.880:8): avc: denied { kernel } for
pid=298 comm="syz-executor372" scontext=system_u:system_r:kernel_t:s0
tcontext=system_u:system_r:kernel_t:s0 tclass=perf_event permissive=1
PMD 800fffffe29ff062
Oops: 0000 [#1] PREEMPT SMP KASAN PTI
CPU: 0 PID: 298 Comm: syz-executor372 Not tainted 6.7.0 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.13.0-1ubuntu1.1 04/01/2014
RIP: 0010:arch_adjust_kprobe_addr+0x42/0x180 arch/x86/kernel/kprobes/core.c:338
Code: 48 89 ea 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 0f b6 14 02
48 89 e8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 09 01 00 00 <44> 8b
6d 00 41 81 fd 66 0f 1f 00 74 18 e8 3c e5 30 00 41 81 e5 ff
RSP: 0018:ffff888112af7a68 EFLAGS: 00010246
RAX: 0000000000000003 RBX: 0000000000000000 RCX: ffffffff902cd938
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff95003e80
RBP: ffffffff95003e80 R08: ffff8881bec62da8 R09: ffffffff930000eb
R10: ffffffff906e4e58 R11: ffffffff92f009b3 R12: ffff888112af7b70
R13: ffff888107f5e258 R14: ffff88810124c6f0 R15: 0000000000000001
FS: 000055555555e880(0000) GS:ffff8881c0000000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffff95003e80 CR3: 00000001036da005 CR4: 0000000000770ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
<TASK>
_kprobe_addr+0x10e/0x140 kernel/kprobes.c:1479
register_kprobe+0xe0/0x15b0 kernel/kprobes.c:1622
__register_trace_kprobe kernel/trace/trace_kprobe.c:510 [inline]
__register_trace_kprobe+0x233/0x2a0 kernel/trace/trace_kprobe.c:478
create_local_trace_kprobe+0x209/0x370 kernel/trace/trace_kprobe.c:1821
perf_kprobe_init+0xed/0x1b0 kernel/trace/trace_event_perf.c:267
perf_kprobe_event_init+0xcc/0x180 kernel/events/core.c:10334
perf_try_init_event+0x10d/0x4e0 kernel/events/core.c:11650
perf_init_event kernel/events/core.c:11720 [inline]
perf_event_alloc kernel/events/core.c:12000 [inline]
perf_event_alloc+0xded/0x3310 kernel/events/core.c:11866
__do_sys_perf_event_open+0x328/0x1d50 kernel/events/core.c:12507
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0x43/0xf0 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x6f/0x77
RIP: 0033:0x7f92060ecb4d
Code: 28 c3 e8 36 29 00 00 66 0f 1f 44 00 00 f3 0f 1e fa 48 89 f8 48
89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d
01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fff2df5c268 EFLAGS: 00000246 ORIG_RAX: 000000000000012a
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f92060ecb4d
RDX: 0000000000000000 RSI: 00000000ffffffff RDI: 0000000020001200
RBP: 00007f92060a6500 R08: 0000000000000000 R09: 0000000000000000
R10: 00000000ffffffff R11: 0000000000000246 R12: 00007f92060a65a0
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
</TASK>
Modules linked in:
CR2: ffffffff95003e80
---[ end trace 0000000000000000 ]---
RIP: 0010:arch_adjust_kprobe_addr+0x42/0x180 arch/x86/kernel/kprobes/core.c:338
Code: 48 89 ea 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 0f b6 14 02
48 89 e8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 09 01 00 00 <44> 8b
6d 00 41 81 fd 66 0f 1f 00 74 18 e8 3c e5 30 00 41 81 e5 ff
RSP: 0018:ffff888112af7a68 EFLAGS: 00010246
RAX: 0000000000000003 RBX: 0000000000000000 RCX: ffffffff902cd938
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff95003e80
RBP: ffffffff95003e80 R08: ffff8881bec62da8 R09: ffffffff930000eb
R10: ffffffff906e4e58 R11: ffffffff92f009b3 R12: ffff888112af7b70
R13: ffff888107f5e258 R14: ffff88810124c6f0 R15: 0000000000000001
FS: 000055555555e880(0000) GS:ffff8881c0000000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffff95003e80 CR3: 00000001036da005 CR4: 0000000000770ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
note: syz-executor372[298] exited with irqs disabled
----------------
Code disassembly (best guess):
0: 48 89 ea mov %rbp,%rdx
3: 48 b8 00 00 00 00 00 movabs $0xdffffc0000000000,%rax
a: fc ff df
d: 48 c1 ea 03 shr $0x3,%rdx
11: 0f b6 14 02 movzbl (%rdx,%rax,1),%edx
15: 48 89 e8 mov %rbp,%rax
18: 83 e0 07 and $0x7,%eax
1b: 83 c0 03 add $0x3,%eax
1e: 38 d0 cmp %dl,%al
20: 7c 08 jl 0x2a
22: 84 d2 test %dl,%dl
24: 0f 85 09 01 00 00 jne 0x133
* 2a: 44 8b 6d 00 mov 0x0(%rbp),%r13d <-- trapping instruction
2e: 41 81 fd 66 0f 1f 00 cmp $0x1f0f66,%r13d
35: 74 18 je 0x4f
37: e8 3c e5 30 00 callq 0x30e578
3c: 41 rex.B
3d: 81 .byte 0x81
3e: e5 ff in $0xff,%eax


2024-03-14 15:08:12

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: BUG: unable to handle kernel paging request in arch_adjust_kprobe_addr

Hi,

Thanks for reporting the bug. I confirmed it and found a bug.

----
/* If x86 supports IBT (ENDBR) it must be skipped. */
kprobe_opcode_t *arch_adjust_kprobe_addr(unsigned long addr, unsigned long offset,
bool *on_func_entry)
{
if (is_endbr(*(u32 *)addr)) {
^^^^^^^^^^^^^^^^^
----

Actually, arch_adjust_kprobe_addr() is called before safety check of the
address. So we should treat the @addr as unsafe address.

Let me fix that.

Thank you,


On Wed, 13 Mar 2024 10:14:09 +0800
cheung wall <[email protected]> wrote:

> Hello,
>
>
>
> when using Healer to fuzz the latest Linux Kernel, the following crash
>
> was triggered on:
>
>
>
> HEAD commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a (tag: v6.7)
>
> git tree: upstream
>
> console output:
> *https://drive.google.com/file/d/15ygRHkG5dwbVMtPDCBx1FKhTULSlrXki/view?usp=drive_link*
> <https://drive.google.com/file/d/15ygRHkG5dwbVMtPDCBx1FKhTULSlrXki/view?usp=drive_link>
>
> kernel config:
> *https://drive.google.com/file/d/1odoVJXVajqeUhF0bpFlv3ieTNwpgNdAl/view?usp=drive_link*
> <https://drive.google.com/file/d/1odoVJXVajqeUhF0bpFlv3ieTNwpgNdAl/view?usp=drive_link>
>
> C reproducer:
> *https://drive.google.com/file/d/1hYKj4Xanb09-3gsIRq3ZLvEhkno49NtP/view?usp=drive_link*
> <https://drive.google.com/file/d/1hYKj4Xanb09-3gsIRq3ZLvEhkno49NtP/view?usp=drive_link>
>
> Syzlang reproducer:
> https://drive.google.com/file/d/1YIN_c_-kT5De7-Z80nWImXyqW7rT2fPf/view?usp=drive_link
>
>
>
> If you fix this issue, please add the following tag to the commit:
>
> Reported-by: Qiang Zhang <*[email protected]* <[email protected]>>
>
> *----------------------------------------------------------*
>
>
>
> BUG: unable to handle page fault for address: ffffffff95003e80
> audit: type=1400 audit(1710291918.880:7): avc: denied { open } for
> pid=298 comm="syz-executor372" scontext=system_u:system_r:kernel_t:s0
> tcontext=system_u:system_r:kernel_t:s0 tclass=perf_event permissive=1
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 1c4a7067 P4D 1c4a7067 PUD 1c4a8063
> audit: type=1400 audit(1710291918.880:8): avc: denied { kernel } for
> pid=298 comm="syz-executor372" scontext=system_u:system_r:kernel_t:s0
> tcontext=system_u:system_r:kernel_t:s0 tclass=perf_event permissive=1
> PMD 800fffffe29ff062
> Oops: 0000 [#1] PREEMPT SMP KASAN PTI
> CPU: 0 PID: 298 Comm: syz-executor372 Not tainted 6.7.0 #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.13.0-1ubuntu1.1 04/01/2014
> RIP: 0010:arch_adjust_kprobe_addr+0x42/0x180
> arch/x86/kernel/kprobes/core.c:338
> Code: 48 89 ea 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 0f b6 14 02 48 89
> e8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 09 01 00 00 <44> 8b 6d 00 41
> 81 fd 66 0f 1f 00 74 18 e8 3c e5 30 00 41 81 e5 ff
> RSP: 0018:ffff888112af7a68 EFLAGS: 00010246
> RAX: 0000000000000003 RBX: 0000000000000000 RCX: ffffffff902cd938
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff95003e80
> RBP: ffffffff95003e80 R08: ffff8881bec62da8 R09: ffffffff930000eb
> R10: ffffffff906e4e58 R11: ffffffff92f009b3 R12: ffff888112af7b70
> R13: ffff888107f5e258 R14: ffff88810124c6f0 R15: 0000000000000001
> FS: 000055555555e880(0000) GS:ffff8881c0000000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffff95003e80 CR3: 00000001036da005 CR4: 0000000000770ef0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> PKRU: 55555554
> Call Trace:
> <TASK>
> _kprobe_addr+0x10e/0x140 kernel/kprobes.c:1479
> register_kprobe+0xe0/0x15b0 kernel/kprobes.c:1622
> __register_trace_kprobe kernel/trace/trace_kprobe.c:510 [inline]
> __register_trace_kprobe+0x233/0x2a0 kernel/trace/trace_kprobe.c:478
> create_local_trace_kprobe+0x209/0x370 kernel/trace/trace_kprobe.c:1821
> perf_kprobe_init+0xed/0x1b0 kernel/trace/trace_event_perf.c:267
> perf_kprobe_event_init+0xcc/0x180 kernel/events/core.c:10334
> perf_try_init_event+0x10d/0x4e0 kernel/events/core.c:11650
> perf_init_event kernel/events/core.c:11720 [inline]
> perf_event_alloc kernel/events/core.c:12000 [inline]
> perf_event_alloc+0xded/0x3310 kernel/events/core.c:11866
> __do_sys_perf_event_open+0x328/0x1d50 kernel/events/core.c:12507
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0x43/0xf0 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x6f/0x77
> RIP: 0033:0x7f92060ecb4d
> Code: 28 c3 e8 36 29 00 00 66 0f 1f 44 00 00 f3 0f 1e fa 48 89 f8 48 89 f7
> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007fff2df5c268 EFLAGS: 00000246 ORIG_RAX: 000000000000012a
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f92060ecb4d
> RDX: 0000000000000000 RSI: 00000000ffffffff RDI: 0000000020001200
> RBP: 00007f92060a6500 R08: 0000000000000000 R09: 0000000000000000
> R10: 00000000ffffffff R11: 0000000000000246 R12: 00007f92060a65a0
> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> </TASK>
> Modules linked in:
> CR2: ffffffff95003e80
> ---[ end trace 0000000000000000 ]---
> RIP: 0010:arch_adjust_kprobe_addr+0x42/0x180
> arch/x86/kernel/kprobes/core.c:338
> Code: 48 89 ea 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 0f b6 14 02 48 89
> e8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 09 01 00 00 <44> 8b 6d 00 41
> 81 fd 66 0f 1f 00 74 18 e8 3c e5 30 00 41 81 e5 ff
> RSP: 0018:ffff888112af7a68 EFLAGS: 00010246
> RAX: 0000000000000003 RBX: 0000000000000000 RCX: ffffffff902cd938
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff95003e80
> RBP: ffffffff95003e80 R08: ffff8881bec62da8 R09: ffffffff930000eb
> R10: ffffffff906e4e58 R11: ffffffff92f009b3 R12: ffff888112af7b70
> R13: ffff888107f5e258 R14: ffff88810124c6f0 R15: 0000000000000001
> FS: 000055555555e880(0000) GS:ffff8881c0000000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffffffff95003e80 CR3: 00000001036da005 CR4: 0000000000770ef0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> PKRU: 55555554
> note: syz-executor372[298] exited with irqs disabled
> ----------------
> Code disassembly (best guess):
> 0: 48 89 ea mov %rbp,%rdx
> 3: 48 b8 00 00 00 00 00 movabs $0xdffffc0000000000,%rax
> a: fc ff df
> d: 48 c1 ea 03 shr $0x3,%rdx
> 11: 0f b6 14 02 movzbl (%rdx,%rax,1),%edx
> 15: 48 89 e8 mov %rbp,%rax
> 18: 83 e0 07 and $0x7,%eax
> 1b: 83 c0 03 add $0x3,%eax
> 1e: 38 d0 cmp %dl,%al
> 20: 7c 08 jl 0x2a
> 22: 84 d2 test %dl,%dl
> 24: 0f 85 09 01 00 00 jne 0x133
> * 2a: 44 8b 6d 00 mov 0x0(%rbp),%r13d <-- trapping instruction
> 2e: 41 81 fd 66 0f 1f 00 cmp $0x1f0f66,%r13d
> 35: 74 18 je 0x4f
> 37: e8 3c e5 30 00 callq 0x30e578
> 3c: 41 rex.B
> 3d: 81 .byte 0x81
> 3e: e5 ff in $0xff,%eax


--
Masami Hiramatsu (Google) <[email protected]>

2024-03-14 15:12:55

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH] kprobes/x86: Use copy_from_kernel_nofault() to read from unsafe address

From: Masami Hiramatsu (Google) <[email protected]>

Read from an unsafe address with copy_from_kernel_nofault() in
arch_adjust_kprobe_addr() because this function is used before checking
the address is in text or not. Syzcaller bot found a bug and reported
the case if user specifies inaccessible data area,
arch_adjust_kprobe_addr() will cause a kernel panic.


Reported-by: Qiang Zhang <[email protected]>
Closes: https://lore.kernel.org/all/CAKHoSAs2rof6vQVBw_Lg_j3QNku0CANZR2qmy4eT7R5Lo8MFbg@mail.gmail.com/
Fixes: cc66bb914578 ("x86/ibt,kprobes: Cure sym+0 equals fentry woes")
Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
---
arch/x86/kernel/kprobes/core.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index a0ce46c0a2d8..a885eea3bd34 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -335,7 +335,16 @@ static int can_probe(unsigned long paddr)
kprobe_opcode_t *arch_adjust_kprobe_addr(unsigned long addr, unsigned long offset,
bool *on_func_entry)
{
- if (is_endbr(*(u32 *)addr)) {
+ u32 insn;
+
+ /*
+ * Since addr is not guaranteed to be safely accessed yet, use
+ * copy_from_kernel_nofault() to get the instruction.
+ */
+ if (copy_from_kernel_nofault(&insn, (void *)addr, sizeof(u32)))
+ return 0;
+
+ if (is_endbr(insn)) {
*on_func_entry = !offset || offset == 4;
if (*on_func_entry)
offset = 4;


2024-03-14 15:15:33

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH] kprobes/x86: Use copy_from_kernel_nofault() to read from unsafe address

On Fri, 15 Mar 2024 00:12:30 +0900
"Masami Hiramatsu (Google)" <[email protected]> wrote:

> From: Masami Hiramatsu (Google) <[email protected]>
>
> Read from an unsafe address with copy_from_kernel_nofault() in
> arch_adjust_kprobe_addr() because this function is used before checking
> the address is in text or not. Syzcaller bot found a bug and reported
> the case if user specifies inaccessible data area,
> arch_adjust_kprobe_addr() will cause a kernel panic.
>
>
> Reported-by: Qiang Zhang <[email protected]>
> Closes: https://lore.kernel.org/all/CAKHoSAs2rof6vQVBw_Lg_j3QNku0CANZR2qmy4eT7R5Lo8MFbg@mail.gmail.com/
> Fixes: cc66bb914578 ("x86/ibt,kprobes: Cure sym+0 equals fentry woes")
> Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
> ---
> arch/x86/kernel/kprobes/core.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index a0ce46c0a2d8..a885eea3bd34 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -335,7 +335,16 @@ static int can_probe(unsigned long paddr)
> kprobe_opcode_t *arch_adjust_kprobe_addr(unsigned long addr, unsigned long offset,
> bool *on_func_entry)
> {
> - if (is_endbr(*(u32 *)addr)) {
> + u32 insn;
> +
> + /*
> + * Since addr is not guaranteed to be safely accessed yet, use
> + * copy_from_kernel_nofault() to get the instruction.
> + */
> + if (copy_from_kernel_nofault(&insn, (void *)addr, sizeof(u32)))
> + return 0;

Oops, it should be NULL.

> +
> + if (is_endbr(insn)) {
> *on_func_entry = !offset || offset == 4;
> if (*on_func_entry)
> offset = 4;
>


--
Masami Hiramatsu (Google) <[email protected]>

2024-03-14 15:17:45

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH v2] kprobes/x86: Use copy_from_kernel_nofault() to read from unsafe address

From: Masami Hiramatsu (Google) <[email protected]>

Read from an unsafe address with copy_from_kernel_nofault() in
arch_adjust_kprobe_addr() because this function is used before checking
the address is in text or not. Syzcaller bot found a bug and reported
the case if user specifies inaccessible data area,
arch_adjust_kprobe_addr() will cause a kernel panic.

Reported-by: Qiang Zhang <[email protected]>
Closes: https://lore.kernel.org/all/CAKHoSAs2rof6vQVBw_Lg_j3QNku0CANZR2qmy4eT7R5Lo8MFbg@mail.gmail.com/
Fixes: cc66bb914578 ("x86/ibt,kprobes: Cure sym+0 equals fentry woes")
Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
---
Changes in v2:
- Fix to return NULL if failed to access the address.
---
arch/x86/kernel/kprobes/core.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index a0ce46c0a2d8..95e4ebe5d514 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -335,7 +335,16 @@ static int can_probe(unsigned long paddr)
kprobe_opcode_t *arch_adjust_kprobe_addr(unsigned long addr, unsigned long offset,
bool *on_func_entry)
{
- if (is_endbr(*(u32 *)addr)) {
+ u32 insn;
+
+ /*
+ * Since addr is not guaranteed to be safely accessed yet, use
+ * copy_from_kernel_nofault() to get the instruction.
+ */
+ if (copy_from_kernel_nofault(&insn, (void *)addr, sizeof(u32)))
+ return NULL;
+
+ if (is_endbr(insn)) {
*on_func_entry = !offset || offset == 4;
if (*on_func_entry)
offset = 4;


2024-03-15 01:13:22

by Jinghao Jia

[permalink] [raw]
Subject: Re: [PATCH v2] kprobes/x86: Use copy_from_kernel_nofault() to read from unsafe address

On 3/14/24 10:17, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <[email protected]>
>
> Read from an unsafe address with copy_from_kernel_nofault() in
> arch_adjust_kprobe_addr() because this function is used before checking
> the address is in text or not. Syzcaller bot found a bug and reported
> the case if user specifies inaccessible data area,
> arch_adjust_kprobe_addr() will cause a kernel panic.

IMHO there is a check on the address in kallsyms_lookup_size_offset to see
if it is a kernel text address before arch_adjust_kprobe_addr is invoked.

The call chain is:

register_kprobe()
_kprobe_addr()
kallsyms_lookup_size_offset() <- check on addr is here
arch_adjust_kprobe_addr()

I wonder why this check was not able to capture the problem in this bug
report (I cannot reproduce it locally).

Thanks,
--Jinghao

>
> Reported-by: Qiang Zhang <[email protected]>
> Closes: https://urldefense.com/v3/__https://lore.kernel.org/all/CAKHoSAs2rof6vQVBw_Lg_j3QNku0CANZR2qmy4eT7R5Lo8MFbg@mail.gmail.com/__;!!DZ3fjg!_C9Dn6-GBlkyS2z34bDUBsEXkTkgWp45MNrd4Rl5I5slz2A3SrurXOxKupsxLMqxC2BMiySCTfB2-5fPhkLP1g$
> Fixes: cc66bb914578 ("x86/ibt,kprobes: Cure sym+0 equals fentry woes")
> Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
> ---
> Changes in v2:
> - Fix to return NULL if failed to access the address.
> ---
> arch/x86/kernel/kprobes/core.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index a0ce46c0a2d8..95e4ebe5d514 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -335,7 +335,16 @@ static int can_probe(unsigned long paddr)
> kprobe_opcode_t *arch_adjust_kprobe_addr(unsigned long addr, unsigned long offset,
> bool *on_func_entry)
> {
> - if (is_endbr(*(u32 *)addr)) {
> + u32 insn;
> +
> + /*
> + * Since addr is not guaranteed to be safely accessed yet, use
> + * copy_from_kernel_nofault() to get the instruction.
> + */
> + if (copy_from_kernel_nofault(&insn, (void *)addr, sizeof(u32)))
> + return NULL;
> +
> + if (is_endbr(insn)) {
> *on_func_entry = !offset || offset == 4;
> if (*on_func_entry)
> offset = 4;
>


Attachments:
OpenPGP_signature.asc (855.00 B)
OpenPGP digital signature

2024-03-16 13:46:49

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH v2] kprobes/x86: Use copy_from_kernel_nofault() to read from unsafe address

On Thu, 14 Mar 2024 18:56:35 -0500
Jinghao Jia <[email protected]> wrote:

> On 3/14/24 10:17, Masami Hiramatsu (Google) wrote:
> > From: Masami Hiramatsu (Google) <[email protected]>
> >
> > Read from an unsafe address with copy_from_kernel_nofault() in
> > arch_adjust_kprobe_addr() because this function is used before checking
> > the address is in text or not. Syzcaller bot found a bug and reported
> > the case if user specifies inaccessible data area,
> > arch_adjust_kprobe_addr() will cause a kernel panic.
>
> IMHO there is a check on the address in kallsyms_lookup_size_offset to see
> if it is a kernel text address before arch_adjust_kprobe_addr is invoked.

Yeah, kallsyms does not ensure the page (especially data) exists.

>
> The call chain is:
>
> register_kprobe()
> _kprobe_addr()
> kallsyms_lookup_size_offset() <- check on addr is here
> arch_adjust_kprobe_addr()
>
> I wonder why this check was not able to capture the problem in this bug
> report (I cannot reproduce it locally).

I could reproduce it locally, it tried to access 'Y' data.
(I attached my .config) And I ensured that this fixed the problem.

The reproduce test actually tried to access initdata area

ffffffff82fb5450 d __alt_reloc_selftest_addr
ffffffff82fb5460 d int3_exception_nb.1
ffffffff82fb5478 d tsc_early_khz
ffffffff82fb547c d io_delay_override
ffffffff82fb5480 d fxregs.0
ffffffff82fb5680 d y <--- access this
ffffffff82fb5688 d x
ffffffff82fb56a0 d xsave_cpuid_features
ffffffff82fb56c8 d l1d_flush_mitigation

`y` is too generic, so check `io_delay_override` which is on the
same page.

$ git grep io_delay_override
arch/x86/kernel/io_delay.c:static int __initdata io_delay_override;

As you can see, it is marked as `__initdata`, and the initdata has been
freed before starting /init.

----
[ 2.679161] Freeing unused kernel image (initmem) memory: 2888K
[ 2.688731] Write protecting the kernel read-only data: 24576k
[ 2.691802] Freeing unused kernel image (rodata/data gap) memory: 1436K
[ 2.746994] x86/mm: Checked W+X mappings: passed, no W+X pages found.
[ 2.748022] x86/mm: Checking user space page tables
[ 2.789520] x86/mm: Checked W+X mappings: passed, no W+X pages found.
[ 2.790527] Run /init as init process
----

So this has been caused because accessing freed initdata.

Thank you,

>
> Thanks,
> --Jinghao
>
> >
> > Reported-by: Qiang Zhang <[email protected]>
> > Closes: https://urldefense.com/v3/__https://lore.kernel.org/all/CAKHoSAs2rof6vQVBw_Lg_j3QNku0CANZR2qmy4eT7R5Lo8MFbg@mail.gmail.com/__;!!DZ3fjg!_C9Dn6-GBlkyS2z34bDUBsEXkTkgWp45MNrd4Rl5I5slz2A3SrurXOxKupsxLMqxC2BMiySCTfB2-5fPhkLP1g$
> > Fixes: cc66bb914578 ("x86/ibt,kprobes: Cure sym+0 equals fentry woes")
> > Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
> > ---
> > Changes in v2:
> > - Fix to return NULL if failed to access the address.
> > ---
> > arch/x86/kernel/kprobes/core.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> > index a0ce46c0a2d8..95e4ebe5d514 100644
> > --- a/arch/x86/kernel/kprobes/core.c
> > +++ b/arch/x86/kernel/kprobes/core.c
> > @@ -335,7 +335,16 @@ static int can_probe(unsigned long paddr)
> > kprobe_opcode_t *arch_adjust_kprobe_addr(unsigned long addr, unsigned long offset,
> > bool *on_func_entry)
> > {
> > - if (is_endbr(*(u32 *)addr)) {
> > + u32 insn;
> > +
> > + /*
> > + * Since addr is not guaranteed to be safely accessed yet, use
> > + * copy_from_kernel_nofault() to get the instruction.
> > + */
> > + if (copy_from_kernel_nofault(&insn, (void *)addr, sizeof(u32)))
> > + return NULL;
> > +
> > + if (is_endbr(insn)) {
> > *on_func_entry = !offset || offset == 4;
> > if (*on_func_entry)
> > offset = 4;
> >


--
Masami Hiramatsu (Google) <[email protected]>


Attachments:
.config (104.91 kB)

2024-03-17 17:25:04

by Jinghao Jia

[permalink] [raw]
Subject: Re: [PATCH v2] kprobes/x86: Use copy_from_kernel_nofault() to read from unsafe address



On 3/16/24 08:46, Masami Hiramatsu (Google) wrote:
> On Thu, 14 Mar 2024 18:56:35 -0500
> Jinghao Jia <[email protected]> wrote:
>
>> On 3/14/24 10:17, Masami Hiramatsu (Google) wrote:
>>> From: Masami Hiramatsu (Google) <[email protected]>
>>>
>>> Read from an unsafe address with copy_from_kernel_nofault() in
>>> arch_adjust_kprobe_addr() because this function is used before checking
>>> the address is in text or not. Syzcaller bot found a bug and reported
>>> the case if user specifies inaccessible data area,
>>> arch_adjust_kprobe_addr() will cause a kernel panic.
>>
>> IMHO there is a check on the address in kallsyms_lookup_size_offset to see
>> if it is a kernel text address before arch_adjust_kprobe_addr is invoked.
>
> Yeah, kallsyms does not ensure the page (especially data) exists.
>
>>
>> The call chain is:
>>
>> register_kprobe()
>> _kprobe_addr()
>> kallsyms_lookup_size_offset() <- check on addr is here
>> arch_adjust_kprobe_addr()
>>
>> I wonder why this check was not able to capture the problem in this bug
>> report (I cannot reproduce it locally).
>
> I could reproduce it locally, it tried to access 'Y' data.
> (I attached my .config) And I ensured that this fixed the problem.
>
> The reproduce test actually tried to access initdata area
>
> ffffffff82fb5450 d __alt_reloc_selftest_addr
> ffffffff82fb5460 d int3_exception_nb.1
> ffffffff82fb5478 d tsc_early_khz
> ffffffff82fb547c d io_delay_override
> ffffffff82fb5480 d fxregs.0
> ffffffff82fb5680 d y <--- access this
> ffffffff82fb5688 d x
> ffffffff82fb56a0 d xsave_cpuid_features
> ffffffff82fb56c8 d l1d_flush_mitigation
>
> `y` is too generic, so check `io_delay_override` which is on the
> same page.
>
> $ git grep io_delay_override
> arch/x86/kernel/io_delay.c:static int __initdata io_delay_override;
>
> As you can see, it is marked as `__initdata`, and the initdata has been
> freed before starting /init.
>
> ----
> [ 2.679161] Freeing unused kernel image (initmem) memory: 2888K
> [ 2.688731] Write protecting the kernel read-only data: 24576k
> [ 2.691802] Freeing unused kernel image (rodata/data gap) memory: 1436K
> [ 2.746994] x86/mm: Checked W+X mappings: passed, no W+X pages found.
> [ 2.748022] x86/mm: Checking user space page tables
> [ 2.789520] x86/mm: Checked W+X mappings: passed, no W+X pages found.
> [ 2.790527] Run /init as init process
> ----
>
> So this has been caused because accessing freed initdata.

Thanks a lot for the explanation! I have confirmed the bug and tested the
patch with CONFIG_DEBUG_PAGEALLOC_ENABLE_DEFAULT=y (which explicitly marks
the init pages as not-present after boot).

Tested-by: Jinghao Jia <[email protected]>

--Jinghao

>
> Thank you,
>
>>
>> Thanks,
>> --Jinghao
>>
>>>
>>> Reported-by: Qiang Zhang <[email protected]>
>>> Closes: https://urldefense.com/v3/__https://lore.kernel.org/all/CAKHoSAs2rof6vQVBw_Lg_j3QNku0CANZR2qmy4eT7R5Lo8MFbg@mail.gmail.com/__;!!DZ3fjg!_C9Dn6-GBlkyS2z34bDUBsEXkTkgWp45MNrd4Rl5I5slz2A3SrurXOxKupsxLMqxC2BMiySCTfB2-5fPhkLP1g$
>>> Fixes: cc66bb914578 ("x86/ibt,kprobes: Cure sym+0 equals fentry woes")
>>> Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
>>> ---
>>> Changes in v2:
>>> - Fix to return NULL if failed to access the address.
>>> ---
>>> arch/x86/kernel/kprobes/core.c | 11 ++++++++++-
>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
>>> index a0ce46c0a2d8..95e4ebe5d514 100644
>>> --- a/arch/x86/kernel/kprobes/core.c
>>> +++ b/arch/x86/kernel/kprobes/core.c
>>> @@ -335,7 +335,16 @@ static int can_probe(unsigned long paddr)
>>> kprobe_opcode_t *arch_adjust_kprobe_addr(unsigned long addr, unsigned long offset,
>>> bool *on_func_entry)
>>> {
>>> - if (is_endbr(*(u32 *)addr)) {
>>> + u32 insn;
>>> +
>>> + /*
>>> + * Since addr is not guaranteed to be safely accessed yet, use
>>> + * copy_from_kernel_nofault() to get the instruction.
>>> + */
>>> + if (copy_from_kernel_nofault(&insn, (void *)addr, sizeof(u32)))
>>> + return NULL;
>>> +
>>> + if (is_endbr(insn)) {
>>> *on_func_entry = !offset || offset == 4;
>>> if (*on_func_entry)
>>> offset = 4;
>>>
>
>


Attachments:
OpenPGP_signature.asc (855.00 B)
OpenPGP digital signature

2024-03-20 08:12:17

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH v2] kprobes/x86: Use copy_from_kernel_nofault() to read from unsafe address

On Sun, 17 Mar 2024 10:53:59 -0500
Jinghao Jia <[email protected]> wrote:

>
>
> On 3/16/24 08:46, Masami Hiramatsu (Google) wrote:
> > On Thu, 14 Mar 2024 18:56:35 -0500
> > Jinghao Jia <[email protected]> wrote:
> >
> >> On 3/14/24 10:17, Masami Hiramatsu (Google) wrote:
> >>> From: Masami Hiramatsu (Google) <[email protected]>
> >>>
> >>> Read from an unsafe address with copy_from_kernel_nofault() in
> >>> arch_adjust_kprobe_addr() because this function is used before checking
> >>> the address is in text or not. Syzcaller bot found a bug and reported
> >>> the case if user specifies inaccessible data area,
> >>> arch_adjust_kprobe_addr() will cause a kernel panic.
> >>
> >> IMHO there is a check on the address in kallsyms_lookup_size_offset to see
> >> if it is a kernel text address before arch_adjust_kprobe_addr is invoked.
> >
> > Yeah, kallsyms does not ensure the page (especially data) exists.
> >
> >>
> >> The call chain is:
> >>
> >> register_kprobe()
> >> _kprobe_addr()
> >> kallsyms_lookup_size_offset() <- check on addr is here
> >> arch_adjust_kprobe_addr()
> >>
> >> I wonder why this check was not able to capture the problem in this bug
> >> report (I cannot reproduce it locally).
> >
> > I could reproduce it locally, it tried to access 'Y' data.
> > (I attached my .config) And I ensured that this fixed the problem.
> >
> > The reproduce test actually tried to access initdata area
> >
> > ffffffff82fb5450 d __alt_reloc_selftest_addr
> > ffffffff82fb5460 d int3_exception_nb.1
> > ffffffff82fb5478 d tsc_early_khz
> > ffffffff82fb547c d io_delay_override
> > ffffffff82fb5480 d fxregs.0
> > ffffffff82fb5680 d y <--- access this
> > ffffffff82fb5688 d x
> > ffffffff82fb56a0 d xsave_cpuid_features
> > ffffffff82fb56c8 d l1d_flush_mitigation
> >
> > `y` is too generic, so check `io_delay_override` which is on the
> > same page.
> >
> > $ git grep io_delay_override
> > arch/x86/kernel/io_delay.c:static int __initdata io_delay_override;
> >
> > As you can see, it is marked as `__initdata`, and the initdata has been
> > freed before starting /init.
> >
> > ----
> > [ 2.679161] Freeing unused kernel image (initmem) memory: 2888K
> > [ 2.688731] Write protecting the kernel read-only data: 24576k
> > [ 2.691802] Freeing unused kernel image (rodata/data gap) memory: 1436K
> > [ 2.746994] x86/mm: Checked W+X mappings: passed, no W+X pages found.
> > [ 2.748022] x86/mm: Checking user space page tables
> > [ 2.789520] x86/mm: Checked W+X mappings: passed, no W+X pages found.
> > [ 2.790527] Run /init as init process
> > ----
> >
> > So this has been caused because accessing freed initdata.
>
> Thanks a lot for the explanation! I have confirmed the bug and tested the
> patch with CONFIG_DEBUG_PAGEALLOC_ENABLE_DEFAULT=y (which explicitly marks
> the init pages as not-present after boot).
>
> Tested-by: Jinghao Jia <[email protected]>
>

Thank you for testing!

Regards,
--
Masami Hiramatsu (Google) <[email protected]>

2024-03-22 11:07:12

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: x86/urgent] kprobes/x86: Use copy_from_kernel_nofault() to read from unsafe address

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

Commit-ID: 4e51653d5d871f40f1bd5cf95cc7f2d8b33d063b
Gitweb: https://git.kernel.org/tip/4e51653d5d871f40f1bd5cf95cc7f2d8b33d063b
Author: Masami Hiramatsu (Google) <[email protected]>
AuthorDate: Fri, 15 Mar 2024 00:17:30 +09:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Fri, 22 Mar 2024 11:40:56 +01:00

kprobes/x86: Use copy_from_kernel_nofault() to read from unsafe address

Read from an unsafe address with copy_from_kernel_nofault() in
arch_adjust_kprobe_addr() because this function is used before checking
the address is in text or not. Syzcaller bot found a bug and reported
the case if user specifies inaccessible data area,
arch_adjust_kprobe_addr() will cause a kernel panic.

[ mingo: Clarified the comment. ]

Fixes: cc66bb914578 ("x86/ibt,kprobes: Cure sym+0 equals fentry woes")
Reported-by: Qiang Zhang <[email protected]>
Tested-by: Jinghao Jia <[email protected]>
Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/171042945004.154897.2221804961882915806.stgit@devnote2
---
arch/x86/kernel/kprobes/core.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 091b3ab..d0e49bd 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -373,7 +373,16 @@ out:
kprobe_opcode_t *arch_adjust_kprobe_addr(unsigned long addr, unsigned long offset,
bool *on_func_entry)
{
- if (is_endbr(*(u32 *)addr)) {
+ u32 insn;
+
+ /*
+ * Since 'addr' is not guaranteed to be safe to access, use
+ * copy_from_kernel_nofault() to read the instruction:
+ */
+ if (copy_from_kernel_nofault(&insn, (void *)addr, sizeof(u32)))
+ return NULL;
+
+ if (is_endbr(insn)) {
*on_func_entry = !offset || offset == 4;
if (*on_func_entry)
offset = 4;