Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210907060040.36222-1-cuibixuan@huawei.com>
In-Reply-To: <20210907060040.36222-1-cuibixuan@huawei.com>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Wed, 8 Sep 2021 21:57:46 -0700
Message-ID: <CAEf4BzbEOpShbC1+iGo5DafFJc6U1gS9ytB2X_X0rqpWUfjbeg@mail.gmail.com>
Subject: Re: [PATCH -next] bpf: Add oversize check before call kvcalloc()
To:     Bixuan Cui <cuibixuan@huawei.com>
Cc:     bpf <bpf@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>, Martin Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        john fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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
>