Re: [syzbot] [mm?] BUG: unable to handle kernel paging request in copy_from_kernel_nofault

Re: [syzbot] [mm?] BUG: unable to handle kernel paging request in copy_from_kernel_nofault

[Jann Horn]

On Tue, Nov 21, 2023 at 6:13 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> On Sun, Nov 19 2023 at 09:53, syzbot wrote:
> > HEAD commit:    1fda5bb66ad8 bpf: Do not allocate percpu memory at init st..
> > git tree:       bpf
> > console+strace: https://syzkaller.appspot.com/x/log.txt?x=12d99420e80000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=2ae0ccd6bfde5eb0
> > dashboard link: https://syzkaller.appspot.com/bug?extid=72aa0161922eba61b50e
> > compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=16dff22f680000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1027dc70e80000
> >
> > Downloadable assets:
> > disk image: https://storage.googleapis.com/syzbot-assets/3e24d257ce8d/disk-1fda5bb6.raw.xz
> > vmlinux: https://storage.googleapis.com/syzbot-assets/eaa9caffb0e4/vmlinux-1fda5bb6.xz
> > kernel image: https://storage.googleapis.com/syzbot-assets/16182bbed726/bzImage-1fda5bb6.xz
> >
> > The issue was bisected to:
> >
> > commit ca247283781d754216395a41c5e8be8ec79a5f1c
> > Author: Andy Lutomirski <luto@kernel.org>
> > Date:   Wed Feb 10 02:33:45 2021 +0000
> >
> >     x86/fault: Don't run fixups for SMAP violations
>
> Reverting that makes the Ooops go away, but wrongly so.
>
> > bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=103d92db680000
> > final oops:     https://syzkaller.appspot.com/x/report.txt?x=123d92db680000
> > console output: https://syzkaller.appspot.com/x/log.txt?x=143d92db680000
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+72aa0161922eba61b50e@syzkaller.appspotmail.com
> > Fixes: ca247283781d ("x86/fault: Don't run fixups for SMAP violations")
> >
> > BUG: unable to handle page fault for address: ffffffffff600000
>
> This is VSYSCALL_ADDR.
>
> So the real question is why the BPF program tries to copy from the
> VSYSCALL page, which is not mapped.

The linked syz repro is:

r0 = bpf$PROG_LOAD(0x5, &(0x7f00000000c0)={0x11, 0xb,
&(0x7f0000000180)=@framed={{}, [@printk={@integer, {}, {}, {}, {},
{0x7, 0x0, 0xb, 0x3, 0x0, 0x0, 0xff600000}, {0x85, 0x0, 0x0, 0x71}}]},
&(0x7f0000000200)='GPL\x00', 0x0, 0x0, 0x0, 0x0, 0x0, '\x00', 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0},
0x90)
bpf$BPF_RAW_TRACEPOINT_OPEN(0x11,
&(0x7f0000000540)={&(0x7f0000000000)='kfree\x00', r0}, 0x10)

So syzkaller generated a BPF tracing program. 0x85 is BPF_JMP |
BPF_CALL, which is used to invoke BPF helpers; 0x71 is 113, which is
the number of the probe_read_kernel helper, which basically takes
arbitrary values as input and casts them to kernel pointers, and then
probe-reads them. And before that is some kinda ALU op with 0xff600000
as immediate.

So it looks like the answer to that question is "the BPF program tries
to copy from the VSYSCALL page because syzkaller decided to write BPF
code that does specifically that, and the BPF helper let it do that".

copy_from_kernel_nofault() does check
copy_from_kernel_nofault_allowed() to make sure the pointer really is
a kernel pointer, and the X86 version of that rejects anything in the
userspace part of the address space. But it does not know about the
vsyscall area.

Re: [syzbot] [mm?] BUG: unable to handle kernel paging request in copy_from_kernel_nofault

[Thomas Gleixner]

On Fri, Dec 08 2023 at 15:11, Jann Horn wrote:
> On Tue, Nov 21, 2023 at 6:13 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>> > BUG: unable to handle page fault for address: ffffffffff600000
>>
>> This is VSYSCALL_ADDR.
>>
>> So the real question is why the BPF program tries to copy from the
>> VSYSCALL page, which is not mapped.
>
> The linked syz repro is:
>
> r0 = bpf$PROG_LOAD(0x5, &(0x7f00000000c0)={0x11, 0xb,
> &(0x7f0000000180)=@framed={{}, [@printk={@integer, {}, {}, {}, {},
> {0x7, 0x0, 0xb, 0x3, 0x0, 0x0, 0xff600000}, {0x85, 0x0, 0x0, 0x71}}]},
> &(0x7f0000000200)='GPL\x00', 0x0, 0x0, 0x0, 0x0, 0x0, '\x00', 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0},
> 0x90)
> bpf$BPF_RAW_TRACEPOINT_OPEN(0x11,
> &(0x7f0000000540)={&(0x7f0000000000)='kfree\x00', r0}, 0x10)
>
> So syzkaller generated a BPF tracing program. 0x85 is BPF_JMP |
> BPF_CALL, which is used to invoke BPF helpers; 0x71 is 113, which is
> the number of the probe_read_kernel helper, which basically takes
> arbitrary values as input and casts them to kernel pointers, and then
> probe-reads them. And before that is some kinda ALU op with 0xff600000
> as immediate.
>
> So it looks like the answer to that question is "the BPF program tries
> to copy from the VSYSCALL page because syzkaller decided to write BPF
> code that does specifically that, and the BPF helper let it do that".

Indeed.

> copy_from_kernel_nofault() does check
> copy_from_kernel_nofault_allowed() to make sure the pointer really is
> a kernel pointer, and the X86 version of that rejects anything in the
> userspace part of the address space. But it does not know about the
> vsyscall area.

That's cureable. Untested fix below.

Thanks for the explanation!

       tglx

---
diff --git a/arch/x86/mm/maccess.c b/arch/x86/mm/maccess.c
index 6993f026adec..8e846833aa37 100644
--- a/arch/x86/mm/maccess.c
+++ b/arch/x86/mm/maccess.c
@@ -3,6 +3,8 @@
 #include <linux/uaccess.h>
 #include <linux/kernel.h>
 
+#include <uapi/asm/vsyscall.h>
+
 #ifdef CONFIG_X86_64
 bool copy_from_kernel_nofault_allowed(const void *unsafe_src, size_t size)
 {
@@ -15,6 +17,9 @@ bool copy_from_kernel_nofault_allowed(const void *unsafe_src, size_t size)
 	if (vaddr < TASK_SIZE_MAX + PAGE_SIZE)
 		return false;
 
+	if ((vaddr & PAGE_MASK) == VSYSCALL_ADDR)
+		return false;
+
 	/*
 	 * Allow everything during early boot before 'x86_virt_bits'
 	 * is initialized.  Needed for instruction decoding in early

Re: [syzbot] [mm?] BUG: unable to handle kernel paging request in copy_from_kernel_nofault

[Hou Tao]

Hi Thomas,

On 12/9/2023 5:01 AM, Thomas Gleixner wrote:
> diff --git a/arch/x86/mm/maccess.c b/arch/x86/mm/maccess.c
> index 6993f026adec..8e846833aa37 100644
> --- a/arch/x86/mm/maccess.c
> +++ b/arch/x86/mm/maccess.c
> @@ -3,6 +3,8 @@
>  #include <linux/uaccess.h>
>  #include <linux/kernel.h>
>  
> +#include <uapi/asm/vsyscall.h>
> +
>  #ifdef CONFIG_X86_64
>  bool copy_from_kernel_nofault_allowed(const void *unsafe_src, size_t size)
>  {
> @@ -15,6 +17,9 @@ bool copy_from_kernel_nofault_allowed(const void *unsafe_src, size_t size)
>  	if (vaddr < TASK_SIZE_MAX + PAGE_SIZE)
>  		return false;
>  
> +	if ((vaddr & PAGE_MASK) == VSYSCALL_ADDR)
> +		return false;
> +
>  	/*
>  	 * Allow everything during early boot before 'x86_virt_bits'
>  	 * is initialized.  Needed for instruction decoding in early

Tested-by: Hou Tao <houtao1@huawei.com>

Could you please post a formal patch for the fix ? The patch fixes the
oops when using bpf_probe_read_kernel() or similar bpf helpers [1] to
read from vsyscall address and you can take my tested-by tag if it is
necessary.

[1]:
https://lore.kernel.org/bpf/CABOYnLynjBoFZOf3Z4BhaZkc5hx_kHfsjiW+UWLoB=w33LvScw@mail.gmail.com/

Re: [syzbot] [mm?] BUG: unable to handle kernel paging request in copy_from_kernel_nofault

[Guilherme G. Piccoli]

Hi Thomas et.al,

I've been testing some syzkaller reports and found 2 other bugs that are
fixed by this patch (see other report at [0]).

So, is there anything we could do in order to get it properly submitted
/ merged? Lemme know if I can help in anything, I could submit it myself
if you prefer, keeping you as author and adding the tested tags.
BTW, feel free to add:

Tested-by: Guilherme G. Piccoli <gpiccoli@igalia.com>


Thanks,

Guilherme


[0] One is internal and the other I already mailed in:
https://lore.kernel.org/lkml/66cb411b-557a-6a70-57c9-457c969fec24@igalia.com/

Re: [syzbot] [mm?] BUG: unable to handle kernel paging request in copy_from_kernel_nofault

[Alexei Starovoitov]

On Thu, Feb 22, 2024 at 6:30 AM Guilherme G. Piccoli
<gpiccoli@igalia.com> wrote:
>
> Hi Thomas et.al,
>
> I've been testing some syzkaller reports and found 2 other bugs that are
> fixed by this patch (see other report at [0]).
>
> So, is there anything we could do in order to get it properly submitted
> / merged? Lemme know if I can help in anything, I could submit it myself
> if you prefer, keeping you as author and adding the tested tags.
> BTW, feel free to add:
>
> Tested-by: Guilherme G. Piccoli <gpiccoli@igalia.com>

The fix is bpf and net trees and probably will be sent to Linus today
as part of net PR.

Re: [syzbot] [mm?] BUG: unable to handle kernel paging request in copy_from_kernel_nofault

[Guilherme G. Piccoli]

On 22/02/2024 13:04, Alexei Starovoitov wrote:
> [...]
> The fix is bpf and net trees and probably will be sent to Linus today
> as part of net PR.

Thanks a lot Alexei!

So, for completeness / archiving, the patch was now split in 2 and the
links are:

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=ee0e39a63b7
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=32019c659ec

Cheers,


Guilherme

Re: [syzbot] [mm?] BUG: unable to handle kernel paging request in copy_from_kernel_nofault

[Alexei Starovoitov]

On Thu, Feb 22, 2024 at 5:09 PM Guilherme G. Piccoli
<gpiccoli@igalia.com> wrote:
>
> On 22/02/2024 13:04, Alexei Starovoitov wrote:
> > [...]
> > The fix is bpf and net trees and probably will be sent to Linus today
> > as part of net PR.
>
> Thanks a lot Alexei!
>
> So, for completeness / archiving, the patch was now split in 2 and the
> links are:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=ee0e39a63b7
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=32019c659ec

Correct.
Please double check with your two syzbot tests.

Re: [syzbot] [mm?] BUG: unable to handle kernel paging request in copy_from_kernel_nofault

[Guilherme G. Piccoli]

On 22/02/2024 22:11, Alexei Starovoitov wrote:
> [...]
> 
> Correct.
> Please double check with your two syzbot tests.

Thanks for confirming!

Tested the fixes both for 5.15.y and 6.1.y - working fine =)
Cheers,


Guilherme