[PATCH] crypto: arm/blake2b - Set FINAL_NONZERO

Herbert Xu posted 1 patch 9 months, 2 weeks ago
[PATCH] crypto: arm/blake2b - Set FINAL_NONZERO
Posted by Herbert Xu 9 months, 2 weeks ago
On Thu, Apr 24, 2025 at 10:39:09PM +0200, Corentin Labbe wrote:
>
> Thanks it fixes my crypto hw devices.
> So Tested-by: Corentin LABBE <clabbe.montjoie@gmail.com>

What about the sha1-ce failure on arm64? Did that go away too?

That didn't seem related to crypto_engine.

> But I still got some crash with blake2b:
> +[   54.348477] alg: shash: blake2b-256-neon test failed (wrong result) on test vector 1, cfg="init+update+final aligned buffer"
> +[   54.348525] alg: self-tests for blake2b-256 using blake2b-256-neon failed (rc=-22)
> +[   54.348536] ------------[ cut here ]------------

OK this is easy, I left out the FINAL_NONZERO bit in the arm patch:

---8<---
Set FINAL_NONZERO as blake2b expects to have at least one byte for
finalisation.

Reported-by: Corentin LABBE <clabbe.montjoie@gmail.com>
Fixes: cc28260ab4fb ("crypto: arm/blake2b - Use API partial block handling")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/arch/arm/crypto/blake2b-neon-glue.c b/arch/arm/crypto/blake2b-neon-glue.c
index 7ae4ba0afe06..2ff443a91724 100644
--- a/arch/arm/crypto/blake2b-neon-glue.c
+++ b/arch/arm/crypto/blake2b-neon-glue.c
@@ -52,7 +52,8 @@ static int crypto_blake2b_finup_neon(struct shash_desc *desc, const u8 *in,
 		.base.cra_driver_name	= driver_name,			\
 		.base.cra_priority	= 200,				\
 		.base.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY |	\
-					  CRYPTO_AHASH_ALG_BLOCK_ONLY,	\
+					  CRYPTO_AHASH_ALG_BLOCK_ONLY |	\
+					  CRYPTO_AHASH_ALG_FINAL_NONZERO, \
 		.base.cra_blocksize	= BLAKE2B_BLOCK_SIZE,		\
 		.base.cra_ctxsize	= sizeof(struct blake2b_tfm_ctx), \
 		.base.cra_module	= THIS_MODULE,			\
-- 
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
Re: [PATCH] crypto: arm/blake2b - Set FINAL_NONZERO
Posted by Corentin Labbe 9 months, 2 weeks ago
Le Fri, Apr 25, 2025 at 11:33:39AM +0800, Herbert Xu a écrit :
> On Thu, Apr 24, 2025 at 10:39:09PM +0200, Corentin Labbe wrote:
> >
> > Thanks it fixes my crypto hw devices.
> > So Tested-by: Corentin LABBE <clabbe.montjoie@gmail.com>
> 
> What about the sha1-ce failure on arm64? Did that go away too?

No they are still there:
[    2.022921] alg: shash: sha1-ce test failed (wrong result) on test vector 0, cfg="init+update+final aligned buffer"
[    2.022950] alg: self-tests for sha1 using sha1-ce failed (rc=-22)
[    2.022957] ------------[ cut here ]------------
[    2.022960] alg: self-tests for sha1 using sha1-ce failed (rc=-22)
[    2.023009] WARNING: CPU: 3 PID: 110 at crypto/testmgr.c:5871 alg_test+0x5e8/0x60c
[    2.023033] Modules linked in:
[    2.023046] CPU: 3 UID: 0 PID: 110 Comm: cryptomgr_test Not tainted 6.15.0-rc1-g583d02477052 #2 PREEMPT 
Setting prompt string to ['-+\\[ end trace \\w* \\]-+[^\\n]*\\r', '/ #', '~ #', 'sh-5.1#', 'Login timed out', 'Login incorrect']
[    2.023057] Hardware name: Pine64 PINE H64 Model A (DT)
[    2.023062] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    2.023071] pc : alg_test+0x5e8/0x60c
[    2.023081] lr : alg_test+0x5e8/0x60c
[    2.023090] sp : ffff80008180bd40
[    2.023093] x29: ffff80008180bde0 x28: 00000000000000bc x27: 0000000000000000
[    2.023106] x26: 00000000ffffffff x25: 00000000ffffffea x24: 0000000000000177
[    2.023118] x23: ffff800081014208 x22: ffff00000152ae80 x21: 000000000500000e
[    2.023130] x20: ffff00000152ae00 x19: ffff800080a26238 x18: 00000000fffffffe
[    2.023142] x17: 2c3020726f746365 x16: 762074736574206e x15: ffff800080fdc47b
[    2.023154] x14: 0000000000000000 x13: ffff800080fdc47f x12: 65742d666c657320
[    2.023165] x11: 0000000000000058 x10: 0000000000000029 x9 : 0000000000000001
[    2.023177] x8 : ffff800080ec3308 x7 : ffff80008180bae0 x6 : 000000000000000c
[    2.023188] x5 : 0000000000000000 x4 : 00000000fffff0b5 x3 : 0000000000000000
[    2.023199] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000099a7000
[    2.023211] Call trace:
[    2.023216]  alg_test+0x5e8/0x60c (P)
[    2.023229]  cryptomgr_test+0x24/0x44
[    2.023240]  kthread+0x12c/0x204
[    2.023253]  ret_from_fork+0x10/0x20
[    2.023267] ---[ end trace 0000000000000000 ]---


> 
> That didn't seem related to crypto_engine.
> 
> > But I still got some crash with blake2b:
> > +[   54.348477] alg: shash: blake2b-256-neon test failed (wrong result) on test vector 1, cfg="init+update+final aligned buffer"
> > +[   54.348525] alg: self-tests for blake2b-256 using blake2b-256-neon failed (rc=-22)
> > +[   54.348536] ------------[ cut here ]------------
> 
> OK this is easy, I left out the FINAL_NONZERO bit in the arm patch:
> 

blake2b crash disappear, so
Tested-by: Corentin LABBE <clabbe.montjoie@gmail.com>

Thanks
[PATCH] crypto: arm64/sha1 - Set finalize for short finup
Posted by Herbert Xu 9 months, 2 weeks ago
On Fri, Apr 25, 2025 at 12:42:17PM +0200, Corentin Labbe wrote:
>
> No they are still there:
> [    2.022921] alg: shash: sha1-ce test failed (wrong result) on test vector 0, cfg="init+update+final aligned buffer"

Thanks, I see the problem now.  The zero-length hash doesn't
set sctx->finalize correctly:

---8<---
Always set sctx->finalize before calling finup as it may not have
been set previously on a short final.

Reported-by: Corentin LABBE <clabbe.montjoie@gmail.com>
Fixes: b97d31100e36 ("crypto: arm64/sha1 - Use API partial block handling")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/arch/arm64/crypto/sha1-ce-glue.c b/arch/arm64/crypto/sha1-ce-glue.c
index 1f8c93fe1e64..65b6980817e5 100644
--- a/arch/arm64/crypto/sha1-ce-glue.c
+++ b/arch/arm64/crypto/sha1-ce-glue.c
@@ -79,8 +79,10 @@ static int sha1_ce_finup(struct shash_desc *desc, const u8 *data,
 		data += len - remain;
 		len = remain;
 	}
-	if (!finalized)
+	if (!finalized) {
+		sctx->finalize = 0;
 		sha1_base_do_finup(desc, data, len, sha1_ce_transform);
+	}
 	return sha1_base_finish(desc, out);
 }
 
-- 
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
Re: [PATCH] crypto: arm64/sha1 - Set finalize for short finup
Posted by Corentin Labbe 9 months, 2 weeks ago
Le Fri, Apr 25, 2025 at 06:58:13PM +0800, Herbert Xu a écrit :
> On Fri, Apr 25, 2025 at 12:42:17PM +0200, Corentin Labbe wrote:
> >
> > No they are still there:
> > [    2.022921] alg: shash: sha1-ce test failed (wrong result) on test vector 0, cfg="init+update+final aligned buffer"
> 
> Thanks, I see the problem now.  The zero-length hash doesn't
> set sctx->finalize correctly:
> 
> ---8<---
> Always set sctx->finalize before calling finup as it may not have
> been set previously on a short final.
> 
> Reported-by: Corentin LABBE <clabbe.montjoie@gmail.com>
> Fixes: b97d31100e36 ("crypto: arm64/sha1 - Use API partial block handling")
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> 

Tested-by: Corentin LABBE <clabbe.montjoie@gmail.com>

Thanks