On 2022/7/21 17:28, Will Deacon wrote:
On Wed, Jul 20, 2022 at 07:37:17PM -0700, Eric Biggers wrote:
On Wed, Jul 20, 2022 at 05:57:30PM +0800, Guozihua (Scott) wrote:
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.
buflen is initialized by neon_poly1305_init(), so there's no issue here.
Ah yes, thanks. I missed that. In which case, for the very little it's
worth:
Acked-by: Will Deacon <will@xxxxxxxxxx>
Herbert, please can you pick this up?
Will
.
Thanks Will.
Herbert, would you please wait for a v3 patch? I'll update the reproduce
example based on Eric's comment.
--
Best
GUO Zihua
