Re: [PATCH v3 2/3] MIPS: crypto: Add crc32 and crc32c hw accelerated module

[James Hogan <jhogan@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3439 bytes --]

On Thu, Feb 15, 2018 at 02:22:14PM -0800, Eric Biggers wrote:
> On Fri, Feb 09, 2018 at 10:11:06PM +0000, James Hogan wrote:
> > +static struct shash_alg crc32_alg = {
> > +	.digestsize		=	CHKSUM_DIGEST_SIZE,
> > +	.setkey			=	chksum_setkey,
> > +	.init			=	chksum_init,
> > +	.update			=	chksum_update,
> > +	.final			=	chksum_final,
> > +	.finup			=	chksum_finup,
> > +	.digest			=	chksum_digest,
> > +	.descsize		=	sizeof(struct chksum_desc_ctx),
> > +	.base			=	{
> > +		.cra_name		=	"crc32",
> > +		.cra_driver_name	=	"crc32-mips-hw",
> > +		.cra_priority		=	300,
> > +		.cra_blocksize		=	CHKSUM_BLOCK_SIZE,
> > +		.cra_alignmask		=	0,
> > +		.cra_ctxsize		=	sizeof(struct chksum_ctx),
> > +		.cra_module		=	THIS_MODULE,
> > +		.cra_init		=	chksum_cra_init,
> > +	}
> > +};
> > +
> > +
> > +static struct shash_alg crc32c_alg = {
> > +       .digestsize             =       CHKSUM_DIGEST_SIZE,
> > +       .setkey                 =       chksum_setkey,
> > +       .init                   =       chksum_init,
> > +       .update                 =       chksumc_update,
> > +       .final                  =       chksumc_final,
> > +       .finup                  =       chksumc_finup,
> > +       .digest                 =       chksumc_digest,
> > +       .descsize               =       sizeof(struct chksum_desc_ctx),
> > +       .base                   =       {
> > +               .cra_name               =       "crc32c",
> > +               .cra_driver_name        =       "crc32c-mips-hw",
> > +               .cra_priority           =       300,
> > +               .cra_blocksize          =       CHKSUM_BLOCK_SIZE,
> > +               .cra_alignmask          =       0,
> > +               .cra_ctxsize            =       sizeof(struct chksum_ctx),
> > +               .cra_module             =       THIS_MODULE,
> > +               .cra_init               =       chksum_cra_init,
> > +       }
> > +};
>
> Does this actually work on the latest kernel?  Now hash algorithms must have
> CRYPTO_ALG_OPTIONAL_KEY in .cra_flags if they have a .setkey method but don't
> require it to be called, otherwise the crypto API will think it's a keyed hash
> and not allow it to be used without a key.  I had to add this flag to the other
> CRC32 and CRC32C algorithms (commit a208fa8f33031).  One of the CRC32C test
> vectors even doesn't set a key so it should be causing the self-tests to fail
> for "crc32c-mips-hw".  (We should add such a test vector for CRC32 too, though.)

Thanks Eric. It does indeed fail now with:

alg: hash: digest failed on test 1 for crc32c-mips-hw: ret=161

I'll squash in the following change:

diff --git a/arch/mips/crypto/crc32-mips.c b/arch/mips/crypto/crc32-mips.c
index 8d4122f37fa5..7d1d2425746f 100644
--- a/arch/mips/crypto/crc32-mips.c
+++ b/arch/mips/crypto/crc32-mips.c
@@ -284,6 +284,7 @@ static struct shash_alg crc32_alg = {
 		.cra_name		=	"crc32",
 		.cra_driver_name	=	"crc32-mips-hw",
 		.cra_priority		=	300,
+		.cra_flags		=	CRYPTO_ALG_OPTIONAL_KEY,
 		.cra_blocksize		=	CHKSUM_BLOCK_SIZE,
 		.cra_alignmask		=	0,
 		.cra_ctxsize		=	sizeof(struct chksum_ctx),
@@ -305,6 +306,7 @@ static struct shash_alg crc32c_alg = {
 		.cra_name		=	"crc32c",
 		.cra_driver_name	=	"crc32c-mips-hw",
 		.cra_priority		=	300,
+		.cra_flags		=	CRYPTO_ALG_OPTIONAL_KEY,
 		.cra_blocksize		=	CHKSUM_BLOCK_SIZE,
 		.cra_alignmask		=	0,
 		.cra_ctxsize		=	sizeof(struct chksum_ctx),

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]