[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZNYSFWA3WNAdRqfg@gondor.apana.org.au>
Date: Fri, 11 Aug 2023 18:48:53 +0800
From: Herbert Xu <herbert@...dor.apana.org.au>
To: Arnd Bergmann <arnd@...db.de>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>
Cc: Arnd Bergmann <arnd@...nel.org>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Ryan Wanner <Ryan.Wanner@...rochip.com>,
Yangtao Li <frank.li@...o.com>, linux-kernel@...r.kernel.org,
"David S . Miller" <davem@...emloft.net>,
Sergiu Moga <sergiu.moga@...rochip.com>,
Ayush Sawal <ayush.sawal@...lsio.com>,
Gaosheng Cui <cuigaosheng1@...wei.com>,
Claudiu Beznea <claudiu.beznea@...rochip.com>,
linux-arm-kernel@...ts.infradead.org, linux-crypto@...r.kernel.org,
Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH 1/2] crypto: drivers - avoid memcpy size warning
On Mon, Aug 07, 2023 at 02:04:05PM +0200, Arnd Bergmann wrote:
>
> 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.
OK I can reproduce this now:
In file included from ../include/linux/string.h:254,
from ../arch/x86/include/asm/page_32.h:18,
from ../arch/x86/include/asm/page.h:14,
from ../arch/x86/include/asm/processor.h:20,
from ../arch/x86/include/asm/timex.h:5,
from ../include/linux/timex.h:67,
from ../include/linux/time32.h:13,
from ../include/linux/time.h:60,
from ../include/linux/stat.h:19,
from ../include/linux/module.h:13,
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 304 and 176 overlaps 1 or more bytes at offset 304 [-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);
| ^~~~~~
../include/linux/fortify-string.h:57:33: error: ‘__builtin_memcpy’ accessing 129 or more bytes at offsets 304 and 176 overlaps 1 or more bytes at offset 304 [-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);
| ^~~~~~
cc1: all warnings being treated as errors
But why are we turning these warnings on if they're giving completely
bogus false positives like this?
struct atmel_sha_hmac_ctx {
struct atmel_sha_ctx base;
struct atmel_sha_hmac_key hkey;
u32 ipad[SHA512_BLOCK_SIZE / sizeof(u32)];
u32 opad[SHA512_BLOCK_SIZE / sizeof(u32)];
atmel_sha_fn_t resume;
};
struct atmel_sha_hmac_ctx *hmac = crypto_ahash_ctx(tfm);
size_t bs = ctx->block_size;
memcpy(hmac->opad, hmac->ipad, bs);
The block_size is set by the algorithm, you can easily grep for
it in atmel-sha.c and the biggest one there is SHA512_BLOCK_SIZE,
which is how big hmac->ipad/hmac->opad are.
So logically this code is perfectly fine.
There is no way for the compiler to know how big ctx->block_size is.
So why do we expect it to make deductions on how big bs can be?
This warning looks broken to me.
It looks like there is already a solution to this though. Just use
unsafe_memcpy and be done with it.
Cheers,
--
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Powered by blists - more mailing lists