| Message ID | 20210907060040.36222-1-cuibixuan@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [-next] bpf: Add oversize check before call kvcalloc() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
| bpf/vmtest-bpf-PR | fail | PR summary |
| bpf/vmtest-bpf | fail | VM_Test |
| bpf/vmtest-bpf-next-PR | success | PR summary |
| bpf/vmtest-bpf-next | success | VM_Test |
On Mon, Sep 6, 2021 at 11:04 PM Bixuan Cui <cuibixuan@huawei.com> wrote: > > Commit 7661809d493b ("mm: don't allow oversized kvmalloc() calls") add the > oversize check. When the allocation is larger than what kmalloc() supports, > the following warning triggered: > > WARNING: CPU: 0 PID: 8408 at mm/util.c:597 kvmalloc_node+0x108/0x110 mm/util.c:597 > Modules linked in: > CPU: 0 PID: 8408 Comm: syz-executor221 Not tainted 5.14.0-syzkaller #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > RIP: 0010:kvmalloc_node+0x108/0x110 mm/util.c:597 > Call Trace: > kvmalloc include/linux/mm.h:806 [inline] > kvmalloc_array include/linux/mm.h:824 [inline] > kvcalloc include/linux/mm.h:829 [inline] > check_btf_line kernel/bpf/verifier.c:9925 [inline] > check_btf_info kernel/bpf/verifier.c:10049 [inline] > bpf_check+0xd634/0x150d0 kernel/bpf/verifier.c:13759 > bpf_prog_load kernel/bpf/syscall.c:2301 [inline] > __sys_bpf+0x11181/0x126e0 kernel/bpf/syscall.c:4587 > __do_sys_bpf kernel/bpf/syscall.c:4691 [inline] > __se_sys_bpf kernel/bpf/syscall.c:4689 [inline] > __x64_sys_bpf+0x78/0x90 kernel/bpf/syscall.c:4689 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x44/0xae > > Reported-by: syzbot+f3e749d4c662818ae439@syzkaller.appspotmail.com > Signed-off-by: Bixuan Cui <cuibixuan@huawei.com> > --- > kernel/bpf/verifier.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 047ac4b4703b..2a3955359156 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -9912,6 +9912,8 @@ static int check_btf_line(struct bpf_verifier_env *env, > nr_linfo = attr->line_info_cnt; > if (!nr_linfo) > return 0; > + if (nr_linfo * sizeof(struct bpf_line_info) > INT_MAX) > + return -EINVAL; I might be missing something, but on 64-bit architecture this can't overflow (because u32 is multiplied by fixed small sizeof()). And on 32-bit architecture if it overflows you won't catch it... So did you mean to do: if (nr_lifo > INT_MAX / sizeof(struct bpf_line_info)) return -EINVAL; ? > > rec_size = attr->line_info_rec_size; > if (rec_size < MIN_BPF_LINEINFO_SIZE || > -- > 2.17.1 >
On 2021/9/9 12:57, Andrii Nakryiko wrote: >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index 047ac4b4703b..2a3955359156 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c >> @@ -9912,6 +9912,8 @@ static int check_btf_line(struct bpf_verifier_env *env, >> nr_linfo = attr->line_info_cnt; >> if (!nr_linfo) >> return 0; >> + if (nr_linfo * sizeof(struct bpf_line_info) > INT_MAX) >> + return -EINVAL; > I might be missing something, but on 64-bit architecture this can't > overflow (because u32 is multiplied by fixed small sizeof()). And on > 32-bit architecture if it overflows you won't catch it... So did you > mean to do: > > if (nr_lifo > INT_MAX / sizeof(struct bpf_line_info)) > return -EINVAL; > > ? On 64-bit architecture, the value of INT_MAX may be equal to the 32-bit. I get the same question: https://stackoverflow.com/questions/9257065/int-max-in-32-bit-vs-64-bit-environment And 'if (nr_lifo > INT_MAX / sizeof(struct bpf_line_info))' is correct on 32-bit architecture ;) Thanks, Bixuan Cui >
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 047ac4b4703b..2a3955359156 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9912,6 +9912,8 @@ static int check_btf_line(struct bpf_verifier_env *env, nr_linfo = attr->line_info_cnt; if (!nr_linfo) return 0; + if (nr_linfo * sizeof(struct bpf_line_info) > INT_MAX) + return -EINVAL; rec_size = attr->line_info_rec_size; if (rec_size < MIN_BPF_LINEINFO_SIZE ||
Commit 7661809d493b ("mm: don't allow oversized kvmalloc() calls") add the oversize check. When the allocation is larger than what kmalloc() supports, the following warning triggered: WARNING: CPU: 0 PID: 8408 at mm/util.c:597 kvmalloc_node+0x108/0x110 mm/util.c:597 Modules linked in: CPU: 0 PID: 8408 Comm: syz-executor221 Not tainted 5.14.0-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:kvmalloc_node+0x108/0x110 mm/util.c:597 Call Trace: kvmalloc include/linux/mm.h:806 [inline] kvmalloc_array include/linux/mm.h:824 [inline] kvcalloc include/linux/mm.h:829 [inline] check_btf_line kernel/bpf/verifier.c:9925 [inline] check_btf_info kernel/bpf/verifier.c:10049 [inline] bpf_check+0xd634/0x150d0 kernel/bpf/verifier.c:13759 bpf_prog_load kernel/bpf/syscall.c:2301 [inline] __sys_bpf+0x11181/0x126e0 kernel/bpf/syscall.c:4587 __do_sys_bpf kernel/bpf/syscall.c:4691 [inline] __se_sys_bpf kernel/bpf/syscall.c:4689 [inline] __x64_sys_bpf+0x78/0x90 kernel/bpf/syscall.c:4689 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae Reported-by: syzbot+f3e749d4c662818ae439@syzkaller.appspotmail.com Signed-off-by: Bixuan Cui <cuibixuan@huawei.com> --- kernel/bpf/verifier.c | 2 ++ 1 file changed, 2 insertions(+)