netdev - Re: mainline build failure due to 281d0c962752 ("fortify: Add Clang support")

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 22 Jun 2022 11:07:40 -0500
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Sudip Mukherjee <sudipm.mukherjee@...il.com>
Cc:     Nathan Chancellor <nathan@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Netdev <netdev@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: mainline build failure due to 281d0c962752 ("fortify: Add Clang support")

On Wed, Jun 22, 2022 at 11:00 AM Sudip Mukherjee
<sudipm.mukherjee@...il.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