Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Fri, 18 Aug 2023 18:25:51 +0800
From:   Herbert Xu <herbert@gondor.apana.org.au>
To:     Arnd Bergmann <arnd@kernel.org>
Cc:     Arnd Bergmann <arnd@arndb.de>,
        "David S. Miller" <davem@davemloft.net>,
        Nicolas Ferre <nicolas.ferre@microchip.com>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Claudiu Beznea <claudiu.beznea@tuxon.dev>,
        Ayush Sawal <ayush.sawal@chelsio.com>,
        Ryan Wanner <Ryan.Wanner@microchip.com>,
        Yangtao Li <frank.li@vivo.com>, Wang Ming <machel@vivo.com>,
        Sergiu Moga <sergiu.moga@microchip.com>,
        Gaosheng Cui <cuigaosheng1@huawei.com>,
        linux-crypto@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [v2] crypto: drivers - avoid memcpy size warning
Message-ID: <ZN9HL5qI5yt/hFnc@gondor.apana.org.au>
References: <20230811134704.3252535-1-arnd@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230811134704.3252535-1-arnd@kernel.org>
Precedence: bulk

On Fri, Aug 11, 2023 at 03:46:33PM +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:
> 
> drivers/crypto/chelsio/chcr_algo.c: In function 'chcr_ahash_setkey':
> include/linux/fortify-string.h:57:33: error: '__builtin_memcpy' accessing 129 or more bytes at offsets 260 and 132 overlaps 1 or more bytes at offset 260 [-Werror=restrict]
> drivers/crypto/bcm/cipher.c: In function 'ahash_hmac_setkey':
> include/linux/fortify-string.h:57:33: error: '__builtin_memcpy' accessing between 129 and 4294967295 bytes at offsets 840 and 712 overlaps between 1 and 4294967167 bytes at offset 840 [-Werror=restrict]
> 
> I don't think it can actually happen because the size is strictly bounded
> to the available block sizes, at most 128 bytes, though inlining decisions
> could lead gcc to not see that.
> 
> Use the unsafe_memcpy() helper instead of memcpy(), with the only difference
> being that this skips the hardening checks that produce the warning.
> 
> Suggested-by: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/crypto/atmel-sha.c         | 3 ++-
>  drivers/crypto/bcm/cipher.c        | 3 ++-
>  drivers/crypto/chelsio/chcr_algo.c | 3 ++-
>  3 files changed, 6 insertions(+), 3 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt