Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-624-g7714e4406d-fm-20230801.001-g7714e440
Mime-Version: 1.0
Message-Id: <d9184ad7-7d34-45bf-81e3-db053bf7425c@app.fastmail.com>
In-Reply-To: <ZMyz27awrVJ8QHzA@gondor.apana.org.au>
References: <20230724135327.1173309-1-arnd@kernel.org>
 <ZMyz27awrVJ8QHzA@gondor.apana.org.au>
Date:   Mon, 07 Aug 2023 14:04:05 +0200
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Herbert Xu" <herbert@gondor.apana.org.au>,
        "Arnd Bergmann" <arnd@kernel.org>
Cc:     "Alexandre Belloni" <alexandre.belloni@bootlin.com>,
        "Ryan Wanner" <Ryan.Wanner@microchip.com>,
        "Yangtao Li" <frank.li@vivo.com>, linux-kernel@vger.kernel.org,
        "David S . Miller" <davem@davemloft.net>,
        "Sergiu Moga" <sergiu.moga@microchip.com>,
        "Ayush Sawal" <ayush.sawal@chelsio.com>,
        "Gaosheng Cui" <cuigaosheng1@huawei.com>,
        "Claudiu Beznea" <claudiu.beznea@microchip.com>,
        linux-arm-kernel@lists.infradead.org, linux-crypto@vger.kernel.org,
        "Kees Cook" <keescook@chromium.org>
Subject: Re: [PATCH 1/2] crypto: drivers - avoid memcpy size warning
Content-Type: text/plain
Precedence: bulk

On Fri, Aug 4, 2023, at 10:16, Herbert Xu wrote:
> On Mon, Jul 24, 2023 at 03:53:01PM +0200, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> Some configurations with gcc-12 or gcc-13 produce a warning for the source
>> and destination of a memcpy() in atmel_sha_hmac_compute_ipad_hash() potentially
>> overlapping:
>> 
>> In file included from include/linux/string.h:254,
>>                  from drivers/crypto/atmel-sha.c:15:
>> drivers/crypto/atmel-sha.c: In function 'atmel_sha_hmac_compute_ipad_hash':
>> include/linux/fortify-string.h:57:33: error: '__builtin_memcpy' accessing 129 or more bytes at offsets 408 and 280 overlaps 1 or more bytes at offset 408 [-Werror=restrict]
>>    57 | #define __underlying_memcpy     __builtin_memcpy
>>       |                                 ^
>> include/linux/fortify-string.h:648:9: note: in expansion of macro '__underlying_memcpy'
>>   648 |         __underlying_##op(p, q, __fortify_size);                        \
>>       |         ^~~~~~~~~~~~~
>> include/linux/fortify-string.h:693:26: note: in expansion of macro '__fortify_memcpy_chk'
>>   693 | #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
>>       |                          ^~~~~~~~~~~~~~~~~~~~
>> drivers/crypto/atmel-sha.c:1773:9: note: in expansion of macro 'memcpy'
>>  1773 |         memcpy(hmac->opad, hmac->ipad, bs);
>>       |         ^~~~~~
>> 
>> The same thing happens in two more drivers that have the same logic:
>
> Please send me the configurations which triggers these warnings.
> As these are false positives, I'd like to enable them only on the
> configurations where they actually cause a problem.

See https://pastebin.com/raw/ip3tfpJF for a config that triggers this
on x86 with the chelsio and atmel drivers. The bcm driver is only
available on arm64, so you won't hit that one here. I also
see this with allmodconfig, as well as defconfig after enabling
CONFIG_FORTIFY_SOURCE and the three crypto drivers.

I see that commit df8fc4e934c12 ("kbuild: Enable -fstrict-flex-arrays=3")
turned on the strict flex-array behavior that triggers the
warning, so this did not show up until linux-6.5-rc1.
In linux-next, I see no other code hit this warning after all
my other patches for it got merged, regardless strict flex
arrays.

At the moment, -Wrestrict is completely disabled in all builds,
so you have to add a patch to enable it in the build system,
this is what I use locally to enable it at the W=1 level,
though you can probably just replace the cc-disable-warning
line with a -Wrestrict line.

     Arnd

--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -49,9 +49,6 @@ KBUILD_CFLAGS += -Wno-pointer-sign
 # globally built with -Wcast-function-type.
 KBUILD_CFLAGS += $(call cc-option, -Wcast-function-type)
 
-# Another good warning that we'll want to enable eventually
-KBUILD_CFLAGS += $(call cc-disable-warning, restrict)
-
 # The allocators already balk at large sizes, so silence the compiler
 # warnings for bounds checks involving those possible values. While
 # -Wno-alloc-size-larger-than would normally be used here, earlier versions
@@ -93,6 +90,7 @@ export KBUILD_EXTRA_WARN
 ifneq ($(findstring 1, $(KBUILD_EXTRA_WARN)),)
 
 KBUILD_CFLAGS += -Wextra -Wunused -Wno-unused-parameter
+KBUILD_CFLAGS += $(call cc-option, -Wrestrict)
 KBUILD_CFLAGS += -Wmissing-format-attribute
 KBUILD_CFLAGS += -Wold-style-definition
 KBUILD_CFLAGS += -Wmissing-include-dirs
@@ -105,6 +103,7 @@ else
 
 # Some diagnostics enabled by default are noisy.
 # Suppress them by using -Wno... except for W=1.
+KBUILD_CFLAGS += $(call cc-disable-warning, restrict)
 KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned)
 
 ifdef CONFIG_CC_IS_CLANG