From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C23B1C47094 for ; Thu, 10 Jun 2021 05:33:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A42FC613E2 for ; Thu, 10 Jun 2021 05:33:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229966AbhFJFft (ORCPT ); Thu, 10 Jun 2021 01:35:49 -0400 Received: from mail-qv1-f42.google.com ([209.85.219.42]:39769 "EHLO mail-qv1-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229692AbhFJFfs (ORCPT ); Thu, 10 Jun 2021 01:35:48 -0400 Received: by mail-qv1-f42.google.com with SMTP id u14so11205909qvq.6 for ; Wed, 09 Jun 2021 22:33:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=OaVwnGtK9jh2uHt5Ipx2vrWPn+KDUOXrS2y79saIzrk=; b=LVuckSqOTYzxTsUyz/Dn8mquXY7TuARv2fApO1OjzaIq/krnrR5d5ShV+lUflmO6Fz sSGKHXNWk/tgIIIHDNg+pGfzKn7Ij6Zvcuw47GW8BrKJG1Pwv/sW2vav62DwI37IgXoh 6CDEJwoa3NhF5YygebCPdY/XS9lc9Qax0HWp5bhaOBYgSmfN9pehgzm+Niu7t8fS7ioW HNf0bjXiJcDigUzABktmGUNYqmhaPmJt91XrVmsJJan64o6ItFsbHqRGu/2doopvFRuN 4pghcbCmmzTM30tSHsmXGlf293hIE0belEUweoRZkaLT9l7XbckAnVFMsZTl56EwNiCK 9LoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OaVwnGtK9jh2uHt5Ipx2vrWPn+KDUOXrS2y79saIzrk=; b=gkLQcmzHoXSWai7ymopmk6FA0P3N7ZSy0SXwDbV4DNzZUIRt2rxe5NBhWXyfnD/Abq A0KIJb5WcFHzuHGEcy0jGY3JmGj2mfkQs197x8tSYHJ3yfTJA7raRiJbOd+OEtsq1IyX Q4Vmakhyg6Qz5pBUk46gbGEiwTLGVYJSdTG3zEo5HRS0MIKTNBf5/QHQ57c7v2Db9QDe 8hM+clT53yju+iSQ1pxnIvktTgobsCUXmF3sNKtMyWG0OaxHd4SFnPyP+iqWRZ4Ci9/N X1k2jq7c6V88F9a7m2tfJ6i0iwNYcfnfUMTt3KDJX+uuPmqQrCbskFrCGJp83h8vC1eD I+mQ== X-Gm-Message-State: AOAM532+JhIhYylzSWg8TBrp1kRcpz/BmlpS/k91pC+8XjpQnH0n3w3t PFujbiJuVmw7dL+GEqSSIaDT9O87rT7Gy87xymzBpQ== X-Google-Smtp-Source: ABdhPJwu7pmXAMs2V9ZXOoDK5dq+cT/fCHwSY6uePqbHFnmbXsUP9knMcgVtIw6TjYPoFfcTLio3a7Cw5RC0WPxmb2I= X-Received: by 2002:a0c:d610:: with SMTP id c16mr3488166qvj.13.1623303156474; Wed, 09 Jun 2021 22:32:36 -0700 (PDT) MIME-Version: 1.0 References: <000000000000c2987605be907e41@google.com> <20210602212726.7-1-fuzzybritches0@gmail.com> <87609-531187-curtm@phaethon> <6a392b66-6f26-4532-d25f-6b09770ce366@fb.com> <202106091119.84A88B6FE7@keescook> <752cb1ad-a0b1-92b7-4c49-bbb42fdecdbe@fb.com> In-Reply-To: <752cb1ad-a0b1-92b7-4c49-bbb42fdecdbe@fb.com> From: Dmitry Vyukov Date: Thu, 10 Jun 2021 07:32:24 +0200 Message-ID: Subject: Re: [PATCH v4] bpf: core: fix shift-out-of-bounds in ___bpf_prog_run To: Yonghong Song Cc: Kees Cook , Alexei Starovoitov , Kurt Manucredo , syzbot+bed360704c521841c85d@syzkaller.appspotmail.com, Andrii Nakryiko , Alexei Starovoitov , bpf , Daniel Borkmann , "David S. Miller" , Jesper Dangaard Brouer , John Fastabend , Martin KaFai Lau , KP Singh , Jakub Kicinski , LKML , Network Development , Song Liu , syzkaller-bugs , nathan@kernel.org, Nick Desaulniers , Clang-Built-Linux ML , linux-kernel-mentees@lists.linuxfoundation.org, Shuah Khan , Greg Kroah-Hartman , Kernel Hardening , kasan-dev Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jun 10, 2021 at 1:40 AM Yonghong Song wrote: > On 6/9/21 11:20 AM, Kees Cook wrote: > > On Mon, Jun 07, 2021 at 09:38:43AM +0200, 'Dmitry Vyukov' via Clang Built Linux wrote: > >> On Sat, Jun 5, 2021 at 9:10 PM Alexei Starovoitov > >> wrote: > >>> On Sat, Jun 5, 2021 at 10:55 AM Yonghong Song wrote: > >>>> On 6/5/21 8:01 AM, Kurt Manucredo wrote: > >>>>> Syzbot detects a shift-out-of-bounds in ___bpf_prog_run() > >>>>> kernel/bpf/core.c:1414:2. > >>>> > >>>> This is not enough. We need more information on why this happens > >>>> so we can judge whether the patch indeed fixed the issue. > >>>> > >>>>> > >>>>> I propose: In adjust_scalar_min_max_vals() move boundary check up to avoid > >>>>> missing them and return with error when detected. > >>>>> > >>>>> Reported-and-tested-by: syzbot+bed360704c521841c85d@syzkaller.appspotmail.com > >>>>> Signed-off-by: Kurt Manucredo > >>>>> --- > >>>>> > >>>>> https://syzkaller.appspot.com/bug?id=edb51be4c9a320186328893287bb30d5eed09231 > >>>>> > >>>>> Changelog: > >>>>> ---------- > >>>>> v4 - Fix shift-out-of-bounds in adjust_scalar_min_max_vals. > >>>>> Fix commit message. > >>>>> v3 - Make it clearer what the fix is for. > >>>>> v2 - Fix shift-out-of-bounds in ___bpf_prog_run() by adding boundary > >>>>> check in check_alu_op() in verifier.c. > >>>>> v1 - Fix shift-out-of-bounds in ___bpf_prog_run() by adding boundary > >>>>> check in ___bpf_prog_run(). > >>>>> > >>>>> thanks > >>>>> > >>>>> kind regards > >>>>> > >>>>> Kurt > >>>>> > >>>>> kernel/bpf/verifier.c | 30 +++++++++--------------------- > >>>>> 1 file changed, 9 insertions(+), 21 deletions(-) > >>>>> > >>>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > >>>>> index 94ba5163d4c5..ed0eecf20de5 100644 > >>>>> --- a/kernel/bpf/verifier.c > >>>>> +++ b/kernel/bpf/verifier.c > >>>>> @@ -7510,6 +7510,15 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, > >>>>> u32_min_val = src_reg.u32_min_value; > >>>>> u32_max_val = src_reg.u32_max_value; > >>>>> > >>>>> + if ((opcode == BPF_LSH || opcode == BPF_RSH || opcode == BPF_ARSH) && > >>>>> + umax_val >= insn_bitness) { > >>>>> + /* Shifts greater than 31 or 63 are undefined. > >>>>> + * This includes shifts by a negative number. > >>>>> + */ > >>>>> + verbose(env, "invalid shift %lld\n", umax_val); > >>>>> + return -EINVAL; > >>>>> + } > >>>> > >>>> I think your fix is good. I would like to move after > >>> > >>> I suspect such change will break valid programs that do shift by register. > >>> > >>>> the following code though: > >>>> > >>>> if (!src_known && > >>>> opcode != BPF_ADD && opcode != BPF_SUB && opcode != BPF_AND) { > >>>> __mark_reg_unknown(env, dst_reg); > >>>> return 0; > >>>> } > >>>> > >>>>> + > >>>>> if (alu32) { > >>>>> src_known = tnum_subreg_is_const(src_reg.var_off); > >>>>> if ((src_known && > >>>>> @@ -7592,39 +7601,18 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, > >>>>> scalar_min_max_xor(dst_reg, &src_reg); > >>>>> break; > >>>>> case BPF_LSH: > >>>>> - if (umax_val >= insn_bitness) { > >>>>> - /* Shifts greater than 31 or 63 are undefined. > >>>>> - * This includes shifts by a negative number. > >>>>> - */ > >>>>> - mark_reg_unknown(env, regs, insn->dst_reg); > >>>>> - break; > >>>>> - } > >>>> > >>>> I think this is what happens. For the above case, we simply > >>>> marks the dst reg as unknown and didn't fail verification. > >>>> So later on at runtime, the shift optimization will have wrong > >>>> shift value (> 31/64). Please correct me if this is not right > >>>> analysis. As I mentioned in the early please write detailed > >>>> analysis in commit log. > >>> > >>> The large shift is not wrong. It's just undefined. > >>> syzbot has to ignore such cases. > >> > >> Hi Alexei, > >> > >> The report is produced by KUBSAN. I thought there was an agreement on > >> cleaning up KUBSAN reports from the kernel (the subset enabled on > >> syzbot at least). > >> What exactly cases should KUBSAN ignore? > >> +linux-hardening/kasan-dev for KUBSAN false positive > > > > Can check_shl_overflow() be used at all? Best to just make things > > readable and compiler-happy, whatever the implementation. :) > > This is not a compile issue. If the shift amount is a constant, > compiler should have warned and user should fix the warning. > > This is because user code has > something like > a << s; > where s is a unknown variable and > verifier just marked the result of a << s as unknown value. > Verifier may not reject the code depending on how a << s result > is used. > > If bpf program writer uses check_shl_overflow() or some kind > of checking for shift value and won't do shifting if the > shifting may cause an undefined result, there should not > be any kubsan warning. I guess the main question: what should happen if a bpf program writer does _not_ use compiler nor check_shl_overflow()? From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 46943C47094 for ; Thu, 10 Jun 2021 05:32:44 +0000 (UTC) Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A0CE1613E2 for ; Thu, 10 Jun 2021 05:32:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A0CE1613E2 Authentication-Results: mail.kernel.org; dmarc=pass (p=none dis=none) header.from=lists.linuxfoundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linux-kernel-mentees-bounces@lists.linuxfoundation.org Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 53A2F60745; Thu, 10 Jun 2021 05:32:43 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 2Tks7IS4ZvBy; Thu, 10 Jun 2021 05:32:42 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTPS id 4DDDB6061F; Thu, 10 Jun 2021 05:32:42 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 2F328C000E; Thu, 10 Jun 2021 05:32:42 +0000 (UTC) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id CCF2BC000B for ; Thu, 10 Jun 2021 05:32:40 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id A43B640599 for ; Thu, 10 Jun 2021 05:32:40 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp4.osuosl.org (amavisd-new); dkim=pass (2048-bit key) header.d=google.com Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id uM-IBa8HioEA for ; Thu, 10 Jun 2021 05:32:38 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.8.0 Received: from mail-qv1-xf31.google.com (mail-qv1-xf31.google.com [IPv6:2607:f8b0:4864:20::f31]) by smtp4.osuosl.org (Postfix) with ESMTPS id 120E340594 for ; Thu, 10 Jun 2021 05:32:37 +0000 (UTC) Received: by mail-qv1-xf31.google.com with SMTP id u14so11205908qvq.6 for ; Wed, 09 Jun 2021 22:32:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=OaVwnGtK9jh2uHt5Ipx2vrWPn+KDUOXrS2y79saIzrk=; b=LVuckSqOTYzxTsUyz/Dn8mquXY7TuARv2fApO1OjzaIq/krnrR5d5ShV+lUflmO6Fz sSGKHXNWk/tgIIIHDNg+pGfzKn7Ij6Zvcuw47GW8BrKJG1Pwv/sW2vav62DwI37IgXoh 6CDEJwoa3NhF5YygebCPdY/XS9lc9Qax0HWp5bhaOBYgSmfN9pehgzm+Niu7t8fS7ioW HNf0bjXiJcDigUzABktmGUNYqmhaPmJt91XrVmsJJan64o6ItFsbHqRGu/2doopvFRuN 4pghcbCmmzTM30tSHsmXGlf293hIE0belEUweoRZkaLT9l7XbckAnVFMsZTl56EwNiCK 9LoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OaVwnGtK9jh2uHt5Ipx2vrWPn+KDUOXrS2y79saIzrk=; b=gTFlSXqr7yMLoeLlwQYJeYslJsGNPDHDVV0wt/k0wtZEizW2+2QspWo9zg4DDR4olK Dp9Y6DeLzNMmXm7ncFCiPg0J6ovH5vzysRzFjw/dqc4y0xHKJ5Wcbqg23NQhVg4wLgNv IWw29q9+lJTPwaVtas/SJgMFbcVSP0Q+zRvLBoNEOHaUEcuOqbGOfZuZbb0aQ97BAFKK PB5jsCSQYxfAK3YVYzFclDObtaHW5eOjiQ/QBVMdvDq6D9z0k0seA5clrxyxCUROy35C 8syOOz2gc9Hojo0ZfaQjyv3XquB2aGiX8rTMAT1Y/dPeAmtKBHTs1ZxiL9wUmJCYgHXt +/AA== X-Gm-Message-State: AOAM531XvcztvWL0FkH6SSUWKDibbP8AzOFcb7nAB37rfqbiqx04fz1V zE+0SVKXvOBJjte9+g52W97YiiGcZ/yrESILN0ZHAw== X-Google-Smtp-Source: ABdhPJwu7pmXAMs2V9ZXOoDK5dq+cT/fCHwSY6uePqbHFnmbXsUP9knMcgVtIw6TjYPoFfcTLio3a7Cw5RC0WPxmb2I= X-Received: by 2002:a0c:d610:: with SMTP id c16mr3488166qvj.13.1623303156474; Wed, 09 Jun 2021 22:32:36 -0700 (PDT) MIME-Version: 1.0 References: <000000000000c2987605be907e41@google.com> <20210602212726.7-1-fuzzybritches0@gmail.com> <87609-531187-curtm@phaethon> <6a392b66-6f26-4532-d25f-6b09770ce366@fb.com> <202106091119.84A88B6FE7@keescook> <752cb1ad-a0b1-92b7-4c49-bbb42fdecdbe@fb.com> In-Reply-To: <752cb1ad-a0b1-92b7-4c49-bbb42fdecdbe@fb.com> Date: Thu, 10 Jun 2021 07:32:24 +0200 Message-ID: Subject: Re: [PATCH v4] bpf: core: fix shift-out-of-bounds in ___bpf_prog_run To: Yonghong Song Cc: Song Liu , Kernel Hardening , Alexei Starovoitov , Andrii Nakryiko , Alexei Starovoitov , Jesper Dangaard Brouer , syzbot+bed360704c521841c85d@syzkaller.appspotmail.com, Daniel Borkmann , John Fastabend , kasan-dev , Clang-Built-Linux ML , Jakub Kicinski , linux-kernel-mentees@lists.linuxfoundation.org, Kees Cook , syzkaller-bugs , KP Singh , nathan@kernel.org, Network Development , Nick Desaulniers , LKML , "David S. Miller" , bpf , Martin KaFai Lau X-BeenThere: linux-kernel-mentees@lists.linuxfoundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Dmitry Vyukov via Linux-kernel-mentees Reply-To: Dmitry Vyukov Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-kernel-mentees-bounces@lists.linuxfoundation.org Sender: "Linux-kernel-mentees" On Thu, Jun 10, 2021 at 1:40 AM Yonghong Song wrote: > On 6/9/21 11:20 AM, Kees Cook wrote: > > On Mon, Jun 07, 2021 at 09:38:43AM +0200, 'Dmitry Vyukov' via Clang Built Linux wrote: > >> On Sat, Jun 5, 2021 at 9:10 PM Alexei Starovoitov > >> wrote: > >>> On Sat, Jun 5, 2021 at 10:55 AM Yonghong Song wrote: > >>>> On 6/5/21 8:01 AM, Kurt Manucredo wrote: > >>>>> Syzbot detects a shift-out-of-bounds in ___bpf_prog_run() > >>>>> kernel/bpf/core.c:1414:2. > >>>> > >>>> This is not enough. We need more information on why this happens > >>>> so we can judge whether the patch indeed fixed the issue. > >>>> > >>>>> > >>>>> I propose: In adjust_scalar_min_max_vals() move boundary check up to avoid > >>>>> missing them and return with error when detected. > >>>>> > >>>>> Reported-and-tested-by: syzbot+bed360704c521841c85d@syzkaller.appspotmail.com > >>>>> Signed-off-by: Kurt Manucredo > >>>>> --- > >>>>> > >>>>> https://syzkaller.appspot.com/bug?id=edb51be4c9a320186328893287bb30d5eed09231 > >>>>> > >>>>> Changelog: > >>>>> ---------- > >>>>> v4 - Fix shift-out-of-bounds in adjust_scalar_min_max_vals. > >>>>> Fix commit message. > >>>>> v3 - Make it clearer what the fix is for. > >>>>> v2 - Fix shift-out-of-bounds in ___bpf_prog_run() by adding boundary > >>>>> check in check_alu_op() in verifier.c. > >>>>> v1 - Fix shift-out-of-bounds in ___bpf_prog_run() by adding boundary > >>>>> check in ___bpf_prog_run(). > >>>>> > >>>>> thanks > >>>>> > >>>>> kind regards > >>>>> > >>>>> Kurt > >>>>> > >>>>> kernel/bpf/verifier.c | 30 +++++++++--------------------- > >>>>> 1 file changed, 9 insertions(+), 21 deletions(-) > >>>>> > >>>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > >>>>> index 94ba5163d4c5..ed0eecf20de5 100644 > >>>>> --- a/kernel/bpf/verifier.c > >>>>> +++ b/kernel/bpf/verifier.c > >>>>> @@ -7510,6 +7510,15 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, > >>>>> u32_min_val = src_reg.u32_min_value; > >>>>> u32_max_val = src_reg.u32_max_value; > >>>>> > >>>>> + if ((opcode == BPF_LSH || opcode == BPF_RSH || opcode == BPF_ARSH) && > >>>>> + umax_val >= insn_bitness) { > >>>>> + /* Shifts greater than 31 or 63 are undefined. > >>>>> + * This includes shifts by a negative number. > >>>>> + */ > >>>>> + verbose(env, "invalid shift %lld\n", umax_val); > >>>>> + return -EINVAL; > >>>>> + } > >>>> > >>>> I think your fix is good. I would like to move after > >>> > >>> I suspect such change will break valid programs that do shift by register. > >>> > >>>> the following code though: > >>>> > >>>> if (!src_known && > >>>> opcode != BPF_ADD && opcode != BPF_SUB && opcode != BPF_AND) { > >>>> __mark_reg_unknown(env, dst_reg); > >>>> return 0; > >>>> } > >>>> > >>>>> + > >>>>> if (alu32) { > >>>>> src_known = tnum_subreg_is_const(src_reg.var_off); > >>>>> if ((src_known && > >>>>> @@ -7592,39 +7601,18 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env, > >>>>> scalar_min_max_xor(dst_reg, &src_reg); > >>>>> break; > >>>>> case BPF_LSH: > >>>>> - if (umax_val >= insn_bitness) { > >>>>> - /* Shifts greater than 31 or 63 are undefined. > >>>>> - * This includes shifts by a negative number. > >>>>> - */ > >>>>> - mark_reg_unknown(env, regs, insn->dst_reg); > >>>>> - break; > >>>>> - } > >>>> > >>>> I think this is what happens. For the above case, we simply > >>>> marks the dst reg as unknown and didn't fail verification. > >>>> So later on at runtime, the shift optimization will have wrong > >>>> shift value (> 31/64). Please correct me if this is not right > >>>> analysis. As I mentioned in the early please write detailed > >>>> analysis in commit log. > >>> > >>> The large shift is not wrong. It's just undefined. > >>> syzbot has to ignore such cases. > >> > >> Hi Alexei, > >> > >> The report is produced by KUBSAN. I thought there was an agreement on > >> cleaning up KUBSAN reports from the kernel (the subset enabled on > >> syzbot at least). > >> What exactly cases should KUBSAN ignore? > >> +linux-hardening/kasan-dev for KUBSAN false positive > > > > Can check_shl_overflow() be used at all? Best to just make things > > readable and compiler-happy, whatever the implementation. :) > > This is not a compile issue. If the shift amount is a constant, > compiler should have warned and user should fix the warning. > > This is because user code has > something like > a << s; > where s is a unknown variable and > verifier just marked the result of a << s as unknown value. > Verifier may not reject the code depending on how a << s result > is used. > > If bpf program writer uses check_shl_overflow() or some kind > of checking for shift value and won't do shifting if the > shifting may cause an undefined result, there should not > be any kubsan warning. I guess the main question: what should happen if a bpf program writer does _not_ use compiler nor check_shl_overflow()? _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees