Re: [PATCH v4 3/8] crypto: hctr2 - Add HCTR2 support

[Eric Biggers <ebiggers@kernel.org>]

One more comment:

On Tue, Apr 12, 2022 at 05:28:11PM +0000, Nathan Huckleberry wrote:
> +/*
> + * Check for a supported set of inner algorithms.
> + * See the comment at the beginning of this file.
> + */
> +static bool hctr2_supported_algorithms(struct skcipher_alg *xctr_alg,
> +				       struct crypto_alg *blockcipher_alg,
> +				       struct shash_alg *polyval_alg)
> +{
> +	if (strncmp(xctr_alg->base.cra_name, "xctr(", 4) != 0)
> +		return false;
> +
> +	if (blockcipher_alg->cra_blocksize != BLOCKCIPHER_BLOCK_SIZE)
> +		return false;
> +
> +	if (strcmp(polyval_alg->base.cra_name, "polyval") != 0)
> +		return false;
> +
> +	return true;
> +}

There are a couple issues here:

- "See the comment at the beginning of this file" doesn't make sense.  I guess
  this was copied from adiantum.c where there is indeed a comment at the
  beginning of the file that explains which "inner" algorithms are allowed.
  However, in hctr2.c there is no such comment (and that's fine; there aren't as
  many special considerations in this area for hctr2 as for adiantum).

- The strncmp() expression uses a string of 5 characters but only compares 4.
  Also this check is redundant anyway, since hctr2_create_common() already does
  this check (correctly, with 5 characters).

How about deleting the hctr2_supported_algorithms() function and putting the 2
needed checks directly in hctr2_create_common()?  I.e., check
blockcipher_alg->cra_blocksize right after the line:

	blockcipher_alg = crypto_spawn_cipher_alg(&ictx->blockcipher_spawn);

... and check polyval_alg->base.cra_name right after the line:

	polyval_alg = crypto_spawn_shash_alg(&ictx->polyval_spawn);

Note, the pr_warn() message "Unsupported HCTR2 instantiation" isn't very
important, and it arguably shouldn't be there since it is user-triggerable.
So you can just delete it.

- Eric

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel