Re: [PATCH v3] crypto: x86/polyval - Fix crashes when keys are not 16-byte aligned

Bruno Goncalves
2022-10-19T12:38:14Z

On Wed, 19 Oct 2022 at 01:04, Nathan Huckleberry <nhuck@google.com> wrote:
>
> crypto_tfm::__crt_ctx is not guaranteed to be 16-byte aligned on x86-64.
> This causes crashes due to movaps instructions in clmul_polyval_update.
>
> Add logic to align polyval_tfm_ctx to 16 bytes.
>
> Fixes: 34f7f6c30112 ("crypto: x86/polyval - Add PCLMULQDQ accelerated implementation of POLYVAL")
> Reported-by: Bruno Goncalves <bgoncalv@redhat.com>
> Signed-off-by: Nathan Huckleberry <nhuck@google.com>
> ---
> arch/x86/crypto/polyval-clmulni_glue.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/crypto/polyval-clmulni_glue.c b/arch/x86/crypto/polyval-clmulni_glue.c
> index b7664d018851..8fa58b0f3cb3 100644
> --- a/arch/x86/crypto/polyval-clmulni_glue.c
> +++ b/arch/x86/crypto/polyval-clmulni_glue.c
> @@ -27,13 +27,17 @@
> #include <asm/cpu_device_id.h>
> #include <asm/simd.h>
>
> +#define POLYVAL_ALIGN 16
> +#define POLYVAL_ALIGN_ATTR __aligned(POLYVAL_ALIGN)
> +#define POLYVAL_ALIGN_EXTRA ((POLYVAL_ALIGN - 1) & ~(CRYPTO_MINALIGN - 1))
> +#define POLYVAL_CTX_SIZE (sizeof(struct polyval_tfm_ctx) + POLYVAL_ALIGN_EXTRA)
> #define NUM_KEY_POWERS 8
>
> struct polyval_tfm_ctx {
> /*
> * These powers must be in the order h^8, ..., h^1.
> */
> - u8 key_powers[NUM_KEY_POWERS][POLYVAL_BLOCK_SIZE];
> + u8 key_powers[NUM_KEY_POWERS][POLYVAL_BLOCK_SIZE] POLYVAL_ALIGN_ATTR;
> };
>
> struct polyval_desc_ctx {
> @@ -45,6 +49,11 @@ asmlinkage void clmul_polyval_update(const struct polyval_tfm_ctx *keys,
> const u8 *in, size_t nblocks, u8 *accumulator);
> asmlinkage void clmul_polyval_mul(u8 *op1, const u8 *op2);
>
> +static inline struct polyval_tfm_ctx *polyval_tfm_ctx(struct crypto_shash *tfm)
> +{
> + return PTR_ALIGN(crypto_shash_ctx(tfm), POLYVAL_ALIGN);
> +}
> +
> static void internal_polyval_update(const struct polyval_tfm_ctx *keys,
> const u8 *in, size_t nblocks, u8 *accumulator)
> {
> @@ -72,7 +81,7 @@ static void internal_polyval_mul(u8 *op1, const u8 *op2)
> static int polyval_x86_setkey(struct crypto_shash *tfm,
> const u8 *key, unsigned int keylen)
> {
> - struct polyval_tfm_ctx *tctx = crypto_shash_ctx(tfm);
> + struct polyval_tfm_ctx *tctx = polyval_tfm_ctx(tfm);
> int i;
>
> if (keylen != POLYVAL_BLOCK_SIZE)
> @@ -102,7 +111,7 @@ static int polyval_x86_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 *tctx = crypto_shash_ctx(desc->tfm);
> + const struct polyval_tfm_ctx *tctx = polyval_tfm_ctx(desc->tfm);
> u8 *pos;
> unsigned int nblocks;
> unsigned int n;
> @@ -143,7 +152,7 @@ static int polyval_x86_update(struct shash_desc *desc,
> static int polyval_x86_final(struct shash_desc *desc, u8 *dst)
> {
> struct polyval_desc_ctx *dctx = shash_desc_ctx(desc);
> - const struct polyval_tfm_ctx *tctx = crypto_shash_ctx(desc->tfm);
> + const struct polyval_tfm_ctx *tctx = polyval_tfm_ctx(desc->tfm);
>
> if (dctx->bytes) {
> internal_polyval_mul(dctx->buffer,
> @@ -167,7 +176,7 @@ static struct shash_alg polyval_alg = {
> .cra_driver_name = "polyval-clmulni",
> .cra_priority = 200,
> .cra_blocksize = POLYVAL_BLOCK_SIZE,
> - .cra_ctxsize = sizeof(struct polyval_tfm_ctx),
> + .cra_ctxsize = POLYVAL_CTX_SIZE,
> .cra_module = THIS_MODULE,
> },
> };
> --
> 2.38.0.413.g74048e4d9e-goog
>

Thanks, this patch worked well for me.

Bruno