Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <YrLtpixBqWDmZT/V@debian> <CAHk-=wiN1ujyVTgyt1GuZiyWAPfpLwwg-FY1V-J56saMyiA1Lg@mail.gmail.com>
 <YrMwXAs9apFRdkVo@debian> <CAHk-=wjmREcirYi4k_CBT+2U8X5VOAjQn0tVD28OdcKJKpA0zg@mail.gmail.com>
 <YrM8kC5zXzZgL/ca@debian>
In-Reply-To: <YrM8kC5zXzZgL/ca@debian>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Wed, 22 Jun 2022 11:07:40 -0500
Message-ID: <CAHk-=wjdjrx_bORk3Th+rk66Rx-U2Zgoz1AOTE_UwVtCpD3N1A@mail.gmail.com>
Subject: Re: mainline build failure due to 281d0c962752 ("fortify: Add Clang support")
To:     Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Cc:     Nathan Chancellor <nathan@kernel.org>,
        Kees Cook <keescook@chromium.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Netdev <netdev@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Jun 22, 2022 at 11:00 AM Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
>
> imho, there is no check for 'i' and it can become more than MAX_FW_TYPE_NUM and
> in that case it will overwrite.

No. That's already checked a few lines before, in the

        if (fw_image->fw_info.fw_section_cnt > MAX_FW_TYPE_NUM) {
                .. error out

path. And fw_section_cnt as a value is an unsigned bitfield of 16
bits, so there's no chance of some kind of integer signedness
confusion.

So clang is just wrong here.

The fact that you can apparently silence the error with an extra bogus
check does hopefully give clang people a clue about *where* clang is
wrong, but it's not an acceptable workaround for the kernel.

We don't write worse source code to make bad compilers happy.

My "use a struct assignment" is more acceptable because at least then
the source code doesn't get worse. It arguably should have been done
that way the whole time, even if 'memcpy()' is the traditional C way
of doing struct assignments (traditional as in "_really_ old
traditional C").

               Linus