| Message ID | 20230811134704.3252535-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [v2] crypto: drivers - avoid memcpy size warning | expand |
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.
diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c index 54fec72dfba27..99a9ff8e743f2 100644 --- a/drivers/crypto/atmel-sha.c +++ b/drivers/crypto/atmel-sha.c @@ -1770,7 +1770,8 @@ static int atmel_sha_hmac_compute_ipad_hash(struct atmel_sha_dev *dd) size_t bs = ctx->block_size; size_t i, num_words = bs / sizeof(u32); - memcpy(hmac->opad, hmac->ipad, bs); + unsafe_memcpy(hmac->opad, hmac->ipad, bs, + "fortified memcpy causes -Wrestrict warning"); for (i = 0; i < num_words; ++i) { hmac->ipad[i] ^= 0x36363636; hmac->opad[i] ^= 0x5c5c5c5c; diff --git a/drivers/crypto/bcm/cipher.c b/drivers/crypto/bcm/cipher.c index 70b911baab26d..4c46357e2570e 100644 --- a/drivers/crypto/bcm/cipher.c +++ b/drivers/crypto/bcm/cipher.c @@ -2397,7 +2397,8 @@ static int ahash_hmac_setkey(struct crypto_ahash *ahash, const u8 *key, memset(ctx->ipad + ctx->authkeylen, 0, blocksize - ctx->authkeylen); ctx->authkeylen = 0; - memcpy(ctx->opad, ctx->ipad, blocksize); + unsafe_memcpy(ctx->opad, ctx->ipad, blocksize, + "fortified memcpy causes -Wrestrict warning"); for (index = 0; index < blocksize; index++) { ctx->ipad[index] ^= HMAC_IPAD_VALUE; diff --git a/drivers/crypto/chelsio/chcr_algo.c b/drivers/crypto/chelsio/chcr_algo.c index 0eade4fa6695b..16298ae4a00bf 100644 --- a/drivers/crypto/chelsio/chcr_algo.c +++ b/drivers/crypto/chelsio/chcr_algo.c @@ -2216,7 +2216,8 @@ static int chcr_ahash_setkey(struct crypto_ahash *tfm, const u8 *key, memcpy(hmacctx->ipad, key, keylen); } memset(hmacctx->ipad + keylen, 0, bs - keylen); - memcpy(hmacctx->opad, hmacctx->ipad, bs); + unsafe_memcpy(hmacctx->opad, hmacctx->ipad, bs, + "fortified memcpy causes -Wrestrict warning"); for (i = 0; i < bs / sizeof(int); i++) { *((unsigned int *)(&hmacctx->ipad) + i) ^= IPAD_DATA;