Re: [RFC PATCH v2 2/7] crypto: polyval - Add POLYVAL support
From: Eric Biggers <ebiggers@kernel.org>
Date: 2022-02-16 23:16:16
Also in:
linux-crypto
On Thu, Feb 10, 2022 at 11:28:07PM +0000, Nathan Huckleberry wrote:
+config CRYPTO_POLYVAL + tristate + select CRYPTO_GF128MUL + select CRYPTO_HASH + help + POLYVAL is the hash function used in HCTR2. It is not a general-purpose + cryptographic hash function.
As with XCTR: as this option is no longer user-selectable, no one will see this help text. I think it should just be removed.
+static int polyval_update(struct shash_desc *desc,
+ const u8 *src, unsigned int srclen)
+{
+ struct polyval_desc_ctx *dctx = shash_desc_ctx(desc);
+ const struct polyval_tfm_ctx *ctx = crypto_shash_ctx(desc->tfm);
+ u8 *dst = dctx->buffer;The dst variable doesn't seem to serve a purpose. It would be clearer to just write dctx->buffer directly (or &dctx->buffer[...], etc).
+ u8 *pos;
+ u8 tmp[POLYVAL_BLOCK_SIZE];
+ int n;
+
+ if (dctx->bytes) {
+ n = min(srclen, dctx->bytes);
+ pos = dst + dctx->bytes - 1;
+
+ dctx->bytes -= n;
+ srclen -= n;
+
+ while (n--)
+ *pos-- ^= *src++;
+
+ if (!dctx->bytes)
+ gf128mul_4k_lle((be128 *)dst, ctx->gf128);
I thought I mentioned this on v1, but the cast to be128 is violating alignment
rules. If the alignment to be128 is needed then a union should be used, e.g.:
struct polyval_desc_ctx {
union {
u8 buffer[POLYVAL_BLOCK_SIZE];
be128 buffer128;
};
u32 bytes;
};
+static int polyval_final(struct shash_desc *desc, u8 *dst)
+{
+ struct polyval_desc_ctx *dctx = shash_desc_ctx(desc);
+ const struct polyval_tfm_ctx *ctx = crypto_shash_ctx(desc->tfm);
+ u8 *buf = dctx->buffer;
+
+ if (dctx->bytes)
+ gf128mul_4k_lle((be128 *)buf, ctx->gf128);
+ dctx->bytes = 0;
+
+ reverse_block(buf);
+ memcpy(dst, buf, POLYVAL_BLOCK_SIZE);
+
+ return 0;
+}Same issues as polyval_update().
quoted hunk ↗ jump to hunk
+diff --git a/include/crypto/polyval.h b/include/crypto/polyval.h new file mode 100644 index 000000000000..fd0c6e124b65 --- /dev/null +++ b/include/crypto/polyval.h@@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Common values for the Polyval hash algorithm + * + * Copyright 2021 Google LLC + */ + +#ifndef _CRYPTO_POLYVAL_H +#define _CRYPTO_POLYVAL_H + +#include <linux/types.h> +#include <linux/crypto.h> + +#define POLYVAL_BLOCK_SIZE 16 +#define POLYVAL_DIGEST_SIZE 16 + +struct polyval_desc_ctx { + u8 buffer[POLYVAL_BLOCK_SIZE]; + u32 bytes; +}; + +#endif
As-is, polyval_desc_ctx is only used by crypto/polyval-generic.c, so it shouldn't be in this header. Either it should be moved to polyval-generic.c, or all implementations should be made to use the same struct. - Eric _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel