Re: [PATCH v2] arm64/crypto: poly1305 fix a read out-of-bound

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2022/7/20 17:41, Will Deacon wrote:
On Tue, Jul 12, 2022 at 03:50:31PM +0800, GUO Zihua wrote:
A kasan error was reported during fuzzing:

[...]

This patch fixes the issue by calling poly1305_init_arm64() instead of
poly1305_init_arch(). This is also the implementation for the same
algorithm on arm platform.

Fixes: f569ca164751 ("crypto: arm64/poly1305 - incorporate OpenSSL/CRYPTOGAMS NEON implementation")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: GUO Zihua <guozihua@xxxxxxxxxx>
---
  arch/arm64/crypto/poly1305-glue.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

I'm not a crypto guy by any stretch of the imagination, but Ard is out
at the moment and this looks like an important fix so I had a crack at
reviewing it.

diff --git a/arch/arm64/crypto/poly1305-glue.c b/arch/arm64/crypto/poly1305-glue.c
index 9c3d86e397bf..1fae18ba11ed 100644
--- a/arch/arm64/crypto/poly1305-glue.c
+++ b/arch/arm64/crypto/poly1305-glue.c
@@ -52,7 +52,7 @@ static void neon_poly1305_blocks(struct poly1305_desc_ctx *dctx, const u8 *src,
  {
  	if (unlikely(!dctx->sset)) {
  		if (!dctx->rset) {
-			poly1305_init_arch(dctx, src);
+			poly1305_init_arm64(&dctx->h, src);
  			src += POLY1305_BLOCK_SIZE;
  			len -= POLY1305_BLOCK_SIZE;
  			dctx->rset = 1;

With this change, we no longer initialise dctx->buflen to 0 as part of the
initialisation. Looking at neon_poly1305_do_update(), I'm a bit worried
that we could land in the 'if (likely(len >= POLY1305_BLOCK_SIZE))' block,
end up with len == 0 and fail to set dctx->buflen. Is this a problem, or is
my ignorance showing?

Will
.

Thanks Will.

I noticed this as well, but I leaved it out so that the behavior is the same as the implementation for arm. The buflen here seems to be used for maintaining any excessive data after the last block, and is zeroed during init. I am not sure why it should be zeroed again during key initialization. Maybe the thought was that the very first block of the data is always used for initializing rset and that is also considered to be the "initialization" process for the algorithm, thus the zeroing of buflen. I could be completely wrong though.

--
Best
GUO Zihua



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux