diff mbox series

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

Message ID 20221018230412.886349-1-nhuck@google.com
State New
Headers
Series [v3] crypto: x86/polyval - Fix crashes when keys are not 16-byte aligned |

Commit Message

Nathan Huckleberry Oct. 18, 2022, 11:04 p.m. UTC 
  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(-)

Comments

Eric Biggers Oct. 18, 2022, 11:12 p.m. UTC | #1 
On Tue, Oct 18, 2022 at 04:04:12PM -0700, Nathan Huckleberry 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>

Reviewed-by: Eric Biggers <ebiggers@google.com>

Please add 'Cc: stable@vger.kernel.org' as well.  (Herbert can do it when
applying this patch, if you don't happen to send another version.)

- Eric
Bruno Goncalves Oct. 19, 2022, 12:13 p.m. UTC | #2 
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
Herbert Xu Oct. 21, 2022, 11:39 a.m. UTC | #3 
On Tue, Oct 18, 2022 at 04:04:12PM -0700, Nathan Huckleberry 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(-)

Patch applied.  Thanks.
diff mbox series

Patch

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,
 	},
 };