crypto: aria: implement aria-avx2 and aria-avx512

PATCH Superseded Taehee Yoo 3 participants 14 messages Nov 6, 2022 – Nov 11, 2022

Subsystems (2)

x86 Crypto API

lore

Share & tools

lore

Taehee Yoo

author

2022-11-06 14:37 UTC #0 lore raw

v3 cover crypto: aria: implement aria-avx2 and aria-avx512

This patchset is to implement aria-avx2 and aria-avx512. There are some differences between aria-avx, aria-avx2, and aria-avx512, but they are not core logic(s-box, diffusion layer).

ARIA-AVX2
It supports 32way parallel processing using 256bit registers. Like ARIA-AVX, it supports both AES-NI based s-box layer algorithm and GFNI based s-box layer algorithm.
These algorithms are the same as ARIA-AVX except that AES-NI doesn't support 256bit registers, so it is used twice.

ARIA-AVX512
It supports 64way parallel processing using 512bit registers. It supports only GFNI based s-box layer algorithm.

Benchmarks with i3-12100
commands: modprobe tcrypt mode=610 num_mb=8192

ARIA-AVX512(128bit and 256bit)
testing speed of multibuffer ecb(aria) (ecb-aria-avx512) encryption
tcrypt: 1 operation in 1504 cycles (1024 bytes)
tcrypt: 1 operation in 4595 cycles (4096 bytes)
tcrypt: 1 operation in 1763 cycles (1024 bytes)
tcrypt: 1 operation in 5540 cycles (4096 bytes)
testing speed of multibuffer ecb(aria) (ecb-aria-avx512) decryption
tcrypt: 1 operation in 1502 cycles (1024 bytes)
tcrypt: 1 operation in 4615 cycles (4096 bytes)
tcrypt: 1 operation in 1759 cycles (1024 bytes)
tcrypt: 1 operation in 5554 cycles (4096 bytes)

ARIA-AVX2 with GFNI(128bit and 256bit)
testing speed of multibuffer ecb(aria) (ecb-aria-avx2) encryption
tcrypt: 1 operation in 2003 cycles (1024 bytes)
tcrypt: 1 operation in 5867 cycles (4096 bytes)
tcrypt: 1 operation in 2358 cycles (1024 bytes)
tcrypt: 1 operation in 7295 cycles (4096 bytes)
testing speed of multibuffer ecb(aria) (ecb-aria-avx2) decryption
tcrypt: 1 operation in 2004 cycles (1024 bytes)
tcrypt: 1 operation in 5956 cycles (4096 bytes)
tcrypt: 1 operation in 2409 cycles (1024 bytes)
tcrypt: 1 operation in 7564 cycles (4096 bytes)

ARIA-AVX with GFNI(128bit and 256bit)
testing speed of multibuffer ecb(aria) (ecb-aria-avx) encryption
tcrypt: 1 operation in 2761 cycles (1024 bytes)
tcrypt: 1 operation in 9390 cycles (4096 bytes)
tcrypt: 1 operation in 3401 cycles (1024 bytes)
tcrypt: 1 operation in 11876 cycles (4096 bytes)
testing speed of multibuffer ecb(aria) (ecb-aria-avx) decryption
tcrypt: 1 operation in 2735 cycles (1024 bytes)
tcrypt: 1 operation in 9424 cycles (4096 bytes)
tcrypt: 1 operation in 3369 cycles (1024 bytes)
tcrypt: 1 operation in 11954 cycles (4096 bytes)

v3:
- Use ARIA_CTX_enc_key, ARIA_CTX_dec_key, and ARIA_CTX_rounds defines.

v2:
- Add new "add keystream array into struct aria_ctx" patch.
- Use keystream array in the aria_ctx instead of stack memory

Taehee Yoo (4):
crypto: aria: add keystream array into struct aria_ctx
crypto: aria: do not use magic number offsets of aria_ctx
crypto: aria: implement aria-avx2
crypto: aria: implement aria-avx512

 arch/x86/crypto/Kconfig                   |   38 +
 arch/x86/crypto/Makefile                  |    6 +
 arch/x86/crypto/aria-aesni-avx-asm_64.S   |   26 +-
 arch/x86/crypto/aria-aesni-avx2-asm_64.S  | 1432 +++++++++++++++++++++
 arch/x86/crypto/aria-avx.h                |   41 +-
 arch/x86/crypto/aria-gfni-avx512-asm_64.S | 1019 +++++++++++++++
 arch/x86/crypto/aria_aesni_avx2_glue.c    |  236 ++++
 arch/x86/crypto/aria_aesni_avx_glue.c     |   30 +-
 arch/x86/crypto/aria_gfni_avx512_glue.c   |  232 ++++
 arch/x86/kernel/asm-offsets.c             |   11 +
 include/crypto/aria.h                     |   24 +
 11 files changed, 3065 insertions(+), 30 deletions(-)
 create mode 100644 arch/x86/crypto/aria-aesni-avx2-asm_64.S
 create mode 100644 arch/x86/crypto/aria-gfni-avx512-asm_64.S
 create mode 100644 arch/x86/crypto/aria_aesni_avx2_glue.c
 create mode 100644 arch/x86/crypto/aria_gfni_avx512_glue.c

Signature · 1 line

2.17.1

Taehee Yoo

author

2022-11-06 14:37 UTC #1 lore raw

PATCH v3 1/4 crypto: aria: add keystream array into struct aria_ctx

avx accelerated aria module used local keystream array. But, keystream array size is too big.
So, it puts the keystream array into struct aria_ctx.

Signed-off-by Taehee Yoo <[email protected]>

---

v3:
- No changes.

v2:
- Patch introduced.

 arch/x86/crypto/aria-avx.h            |  3 ---
 arch/x86/crypto/aria_aesni_avx_glue.c | 24 +++++++++++-------------
 include/crypto/aria.h                 | 11 +++++++++++
 3 files changed, 22 insertions(+), 16 deletions(-)

arch/x86/crypto/aria-avx.h +0 -3

index 01e9a01dc157..afd37af95e58 100644
@@ -4,9 +4,6 @@
 
 #include <linux/types.h>
 
-#define ARIA_AESNI_PARALLEL_BLOCKS 16
-#define ARIA_AESNI_PARALLEL_BLOCK_SIZE  (ARIA_BLOCK_SIZE * 16)
-
 struct aria_avx_ops {
 	void (*aria_encrypt_16way)(const void *ctx, u8 *dst, const u8 *src);
 	void (*aria_decrypt_16way)(const void *ctx, u8 *dst, const u8 *src);

arch/x86/crypto/aria_aesni_avx_glue.c +11 ~~-13~~

index c561ea4fefa5..b122482d0c9d 100644
@@ -86,10 +86,9 @@ static int aria_avx_ctr_encrypt(struct skcipher_request *req)
 		u8 *dst = walk.dst.virt.addr;
 
 		while (nbytes >= ARIA_AESNI_PARALLEL_BLOCK_SIZE) {
-			u8 keystream[ARIA_AESNI_PARALLEL_BLOCK_SIZE];
-
 			kernel_fpu_begin();
-			aria_ops.aria_ctr_crypt_16way(ctx, dst, src, keystream,
+			aria_ops.aria_ctr_crypt_16way(ctx, dst, src,
+						      &ctx->keystream[0],
 						      walk.iv);
 			kernel_fpu_end();
 			dst += ARIA_AESNI_PARALLEL_BLOCK_SIZE;

@@ -98,28 +97,27 @@ static int aria_avx_ctr_encrypt(struct skcipher_request *req)
 		}
 
 		while (nbytes >= ARIA_BLOCK_SIZE) {
-			u8 keystream[ARIA_BLOCK_SIZE];
-
-			memcpy(keystream, walk.iv, ARIA_BLOCK_SIZE);
+			memcpy(&ctx->keystream[0], walk.iv, ARIA_BLOCK_SIZE);
 			crypto_inc(walk.iv, ARIA_BLOCK_SIZE);
 
-			aria_encrypt(ctx, keystream, keystream);
+			aria_encrypt(ctx, &ctx->keystream[0],
+				     &ctx->keystream[0]);
 
-			crypto_xor_cpy(dst, src, keystream, ARIA_BLOCK_SIZE);
+			crypto_xor_cpy(dst, src, &ctx->keystream[0],
+				       ARIA_BLOCK_SIZE);
 			dst += ARIA_BLOCK_SIZE;
 			src += ARIA_BLOCK_SIZE;
 			nbytes -= ARIA_BLOCK_SIZE;
 		}
 
 		if (walk.nbytes == walk.total && nbytes > 0) {
-			u8 keystream[ARIA_BLOCK_SIZE];
-
-			memcpy(keystream, walk.iv, ARIA_BLOCK_SIZE);
+			memcpy(&ctx->keystream[0], walk.iv, ARIA_BLOCK_SIZE);
 			crypto_inc(walk.iv, ARIA_BLOCK_SIZE);
 
-			aria_encrypt(ctx, keystream, keystream);
+			aria_encrypt(ctx, &ctx->keystream[0],
+				     &ctx->keystream[0]);
 
-			crypto_xor_cpy(dst, src, keystream, nbytes);
+			crypto_xor_cpy(dst, src, &ctx->keystream[0], nbytes);
 			dst += nbytes;
 			src += nbytes;
 			nbytes = 0;

include/crypto/aria.h +11 -0

index 254da46cc385..f5c7a87378cd 100644
@@ -31,11 +31,22 @@
 #define ARIA_MAX_RD_KEYS	17
 #define ARIA_RD_KEY_WORDS	(ARIA_BLOCK_SIZE / sizeof(u32))
 
+#define ARIA_AESNI_PARALLEL_BLOCKS 16
+#define ARIA_AESNI_PARALLEL_BLOCK_SIZE  (ARIA_BLOCK_SIZE * 16)
+#if defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64) ||	\
+	defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64_MODULE)
+#define ARIA_KEYSTREAM_SIZE ARIA_AESNI_PARALLEL_BLOCK_SIZE
+#endif
+
 struct aria_ctx {
 	u32 enc_key[ARIA_MAX_RD_KEYS][ARIA_RD_KEY_WORDS];
 	u32 dec_key[ARIA_MAX_RD_KEYS][ARIA_RD_KEY_WORDS];
 	int rounds;
 	int key_length;
+#if defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64) ||	\
+	defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64_MODULE)
+	u8 keystream[ARIA_KEYSTREAM_SIZE];
+#endif
 };
 
 static const u32 s1[256] = {

Signature · 1 line

2.17.1

Herbert Xu

maintainer

2022-11-07 08:48 UTC #2 lore raw

Re: [PATCH v3 1/4] crypto: aria: add keystream array into struct aria_ctx

include/crypto/aria.h line 50

Quoted 11 lines · Taehee Yoo

struct aria_ctx {

include/crypto/aria.h +4 -0

 	u32 enc_key[ARIA_MAX_RD_KEYS][ARIA_RD_KEY_WORDS];
 	u32 dec_key[ARIA_MAX_RD_KEYS][ARIA_RD_KEY_WORDS];
 	int rounds;
 	int key_length;
+#if defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64) ||	\
+	defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64_MODULE)
+	u8 keystream[ARIA_KEYSTREAM_SIZE];
+#endif
 };

The tfm ctx is shared between all users of the tfm. You need something that is private to the request so this needs to be moved into the reqctx.

Cheers,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Taehee Yoo

author

2022-11-07 11:40 UTC #3 lore raw

Re: [PATCH v3 1/4] crypto: aria: add keystream array into struct aria_ctx

Hi Herbert,
Thank you so much for your review!

Quoted 17 lines · Herbert Xu

include/crypto/aria.h line 50
Quoted 11 lines · Taehee Yoo
  struct aria_ctx {
  	u32 enc_key[ARIA_MAX_RD_KEYS][ARIA_RD_KEY_WORDS];
  	u32 dec_key[ARIA_MAX_RD_KEYS][ARIA_RD_KEY_WORDS];
  	int rounds;
  	int key_length;
include/crypto/aria.h +4 -0
+#if defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64) ||	\
+	defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64_MODULE)
+	u8 keystream[ARIA_KEYSTREAM_SIZE];
+#endif
  };
The tfm ctx is shared between all users of the tfm. You need something that is private to the request so this needs to be moved into the reqctx.

Quoted 1 line

Cheers,

Sorry, I think I hadn't understood the point of your previous review correctly. I will move keystream into reqctx instead of the tfm.

Thanks a lot!
Taehee Yoo

Taehee Yoo

author

2022-11-09 13:17 UTC #4 lore raw

Re: [PATCH v3 1/4] crypto: aria: add keystream array into struct aria_ctx

In reply to Herbert Xu

Hi Herbert,

Quoted 17 lines · Herbert Xu

include/crypto/aria.h line 50
Quoted 11 lines · Taehee Yoo
  struct aria_ctx {
  	u32 enc_key[ARIA_MAX_RD_KEYS][ARIA_RD_KEY_WORDS];
  	u32 dec_key[ARIA_MAX_RD_KEYS][ARIA_RD_KEY_WORDS];
  	int rounds;
  	int key_length;
include/crypto/aria.h +4 -0
+#if defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64) ||	\
+	defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64_MODULE)
+	u8 keystream[ARIA_KEYSTREAM_SIZE];
+#endif
  };
The tfm ctx is shared between all users of the tfm. You need something that is private to the request so this needs to be moved into the reqctx.

Quoted 1 line

Cheers,

I have encountered kernel panic(stack-out-of-bounds) while using the reqctx instead of the tfm.

cryptd is used when simd drivers are used.
cryptd_skcipher_encrypt() internally doesn't allocate a request ctx of a child, instead, it uses stack memory with SYNC_SKCIPHER_REQUEST_ON_STACK. It retains only 384 bytes for child request ctx even if a child set a large reqsize value with crypto_skcipher_set_reqsize(). aria-avx2 needs 512 bytes and aria-avx512 needs 1024 bytes.
So, stack-out-of-bounds occurs.

What do you think about using the per-cpu for the keystream array? SIMD functions are called under fpu context.
So, per-cpu keystream will not be used concurrently, it's safe. It also can avoid unnecessary allocation for large temporary memory.

I tested the per-cpu for keystream, and it works well. If you are okay, I would like to submit v4 patch using per-cpu.

Thanks,
Taehee Yoo

Herbert Xu

maintainer

2022-11-10 03:19 UTC #5 lore raw

Re: [PATCH v3 1/4] crypto: aria: add keystream array into struct aria_ctx

On Wed, Nov 09, 2022 at 10:16:58PM +0900, Taehee Yoo wrote:
.

Quoted 10 lines

I have encountered kernel panic(stack-out-of-bounds) while using the reqctx instead of the tfm.
cryptd is used when simd drivers are used. cryptd_skcipher_encrypt() internally doesn't allocate a request ctx of a child, instead, it uses stack memory with SYNC_SKCIPHER_REQUEST_ON_STACK. It retains only 384 bytes for child request ctx even if a child set a large reqsize value with crypto_skcipher_set_reqsize().
aria-avx2 needs 512 bytes and aria-avx512 needs 1024 bytes.
So, stack-out-of-bounds occurs.

That's not good. Let me look into this.

Thanks,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Herbert Xu

maintainer

2022-11-11 09:59 UTC #6 lore raw

crypto: cryptd - Use request context instead of stack for sub-request

In reply to Taehee Yoo

Quoted 11 lines · Taehee Yoo

I have encountered kernel panic(stack-out-of-bounds) while using the reqctx instead of the tfm.
cryptd is used when simd drivers are used. cryptd_skcipher_encrypt() internally doesn't allocate a request ctx of a child, instead, it uses stack memory with SYNC_SKCIPHER_REQUEST_ON_STACK. It retains only 384 bytes for child request ctx even if a child set a large reqsize value with crypto_skcipher_set_reqsize().
aria-avx2 needs 512 bytes and aria-avx512 needs 1024 bytes.
So, stack-out-of-bounds occurs.

OK this is not supposed to happen.

---8<---
cryptd is buggy as it tries to use sync_skcipher without going through the proper sync_skcipher interface. In fact it doesn't even need sync_skcipher since it's already a proper skcipher and can easily access the request context instead of using something off the stack.

Fixes 36b3875a97b8 ("crypto: cryptd - Remove VLA usage of skcipher")

Signed-off-by Herbert Xu <[email protected]>

crypto/cryptd.c +19 ~~-17~~

index 668095eca0fa..ca3a40fc7da9 100644
@@ -68,11 +68,12 @@ struct aead_instance_ctx {
 
 struct cryptd_skcipher_ctx {
 	refcount_t refcnt;
-	struct crypto_sync_skcipher *child;
+	struct crypto_skcipher *child;
 };
 
 struct cryptd_skcipher_request_ctx {
 	crypto_completion_t complete;
+	struct skcipher_request req;
 };
 
 struct cryptd_hash_ctx {

@@ -227,13 +228,13 @@ static int cryptd_skcipher_setkey(struct crypto_skcipher *parent,
 				  const u8 *key, unsigned int keylen)
 {
 	struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(parent);
-	struct crypto_sync_skcipher *child = ctx->child;
+	struct crypto_skcipher *child = ctx->child;
 
-	crypto_sync_skcipher_clear_flags(child, CRYPTO_TFM_REQ_MASK);
-	crypto_sync_skcipher_set_flags(child,
-				       crypto_skcipher_get_flags(parent) &
-					 CRYPTO_TFM_REQ_MASK);
-	return crypto_sync_skcipher_setkey(child, key, keylen);
+	crypto_skcipher_clear_flags(child, CRYPTO_TFM_REQ_MASK);
+	crypto_skcipher_set_flags(child,
+				  crypto_skcipher_get_flags(parent) &
+				  CRYPTO_TFM_REQ_MASK);
+	return crypto_skcipher_setkey(child, key, keylen);
 }
 
 static void cryptd_skcipher_complete(struct skcipher_request *req, int err)

@@ -258,13 +259,13 @@ static void cryptd_skcipher_encrypt(struct crypto_async_request *base,
 	struct cryptd_skcipher_request_ctx *rctx = skcipher_request_ctx(req);
 	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
 	struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
-	struct crypto_sync_skcipher *child = ctx->child;
-	SYNC_SKCIPHER_REQUEST_ON_STACK(subreq, child);
+	struct skcipher_request *subreq = &rctx->req;
+	struct crypto_skcipher *child = ctx->child;
 
 	if (unlikely(err == -EINPROGRESS))
 		goto out;
 
-	skcipher_request_set_sync_tfm(subreq, child);
+	skcipher_request_set_tfm(subreq, child);
 	skcipher_request_set_callback(subreq, CRYPTO_TFM_REQ_MAY_SLEEP,
 				      NULL, NULL);
 	skcipher_request_set_crypt(subreq, req->src, req->dst, req->cryptlen,

@@ -286,13 +287,13 @@ static void cryptd_skcipher_decrypt(struct crypto_async_request *base,
 	struct cryptd_skcipher_request_ctx *rctx = skcipher_request_ctx(req);
 	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
 	struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
-	struct crypto_sync_skcipher *child = ctx->child;
-	SYNC_SKCIPHER_REQUEST_ON_STACK(subreq, child);
+	struct skcipher_request *subreq = &rctx->req;
+	struct crypto_skcipher *child = ctx->child;
 
 	if (unlikely(err == -EINPROGRESS))
 		goto out;
 
-	skcipher_request_set_sync_tfm(subreq, child);
+	skcipher_request_set_tfm(subreq, child);
 	skcipher_request_set_callback(subreq, CRYPTO_TFM_REQ_MAY_SLEEP,
 				      NULL, NULL);
 	skcipher_request_set_crypt(subreq, req->src, req->dst, req->cryptlen,

@@ -343,9 +344,10 @@ static int cryptd_skcipher_init_tfm(struct crypto_skcipher *tfm)
 	if (IS_ERR(cipher))
 		return PTR_ERR(cipher);
 
-	ctx->child = (struct crypto_sync_skcipher *)cipher;
+	ctx->child = cipher;
 	crypto_skcipher_set_reqsize(
-		tfm, sizeof(struct cryptd_skcipher_request_ctx));
+		tfm, sizeof(struct cryptd_skcipher_request_ctx) +
+		     crypto_skcipher_reqsize(cipher));
 	return 0;
 }

@@ -353,7 +355,7 @@ static void cryptd_skcipher_exit_tfm(struct crypto_skcipher *tfm)
 {
 	struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
 
-	crypto_free_sync_skcipher(ctx->child);
+	crypto_free_skcipher(ctx->child);
 }
 
 static void cryptd_skcipher_free(struct skcipher_instance *inst)

@@ -931,7 +933,7 @@ struct crypto_skcipher *cryptd_skcipher_child(struct cryptd_skcipher *tfm)
 {
 	struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(&tfm->base);
 
-	return &ctx->child->base;
+	return ctx->child;
 }
 EXPORT_SYMBOL_GPL(cryptd_skcipher_child);

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Herbert Xu

maintainer

2022-11-11 10:05 UTC #7 lore raw

crypto: skcipher - Allow sync algorithms with large request contexts

Quoted 9 lines · Herbert Xu

cryptd is buggy as it tries to use sync_skcipher without going through the proper sync_skcipher interface. In fact it doesn't even need sync_skcipher since it's already a proper skcipher and can easily access the request context instead of using something off the stack.
Fixes 36b3875a97b8 ("crypto: cryptd - Remove VLA usage of skcipher")
Signed-off-by Herbert Xu <[email protected]>

This won't be enough to allow a sync skcipher that uses more than 384 bytes of request context though as they will still show up when you allocate a sync_skcipher. So we also need this and then you can just set REQSIZE_LARGE on your algorithm and it will work correctly.

---8<---
Some sync algorithms may require a large amount of temporary space during its operations. There is no reason why they should be limited just because some legacy users want to place all temporary data on the stack.

Such algorithms can now set a flag to indicate that they need extra request context, which will cause them to be invisible to users that go through the sync_skcipher interface.

Signed-off-by Herbert Xu <[email protected]>

crypto/skcipher.c +1 -1

index 418211180cee..0ecab31cfe79 100644
@@ -763,7 +763,7 @@ struct crypto_sync_skcipher *crypto_alloc_sync_skcipher(
 	struct crypto_skcipher *tfm;
 
 	/* Only sync algorithms allowed. */
-	mask |= CRYPTO_ALG_ASYNC;
+	mask |= CRYPTO_ALG_ASYNC | CRYPTO_ALG_SKCIPHER_REQSIZE_LARGE;
 
 	tfm = crypto_alloc_tfm(alg_name, &crypto_skcipher_type, type, mask);

include/crypto/internal/skcipher.h +8 -0

index a2339f80a615..2a97540156bb 100644
@@ -14,6 +14,14 @@
 #include <linux/list.h>
 #include <linux/types.h>
 
+/*
+ * Set this if your algorithm is sync but needs a reqsize larger
+ * than MAX_SYNC_SKCIPHER_REQSIZE.
+ *
+ * Reuse bit that is specific to hash algorithms.
+ */
+#define CRYPTO_ALG_SKCIPHER_REQSIZE_LARGE CRYPTO_ALG_OPTIONAL_KEY
+
 struct aead_request;
 struct rtattr;

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Taehee Yoo

author

2022-11-11 15:43 UTC #8 lore raw

Re: crypto: cryptd - Use request context instead of stack for sub-request

In reply to Herbert Xu

Hi Herbert,
Thank you so much for this work!

Quoted 13 lines · Herbert Xu

Quoted 11 lines · Taehee Yoo
I have encountered kernel panic(stack-out-of-bounds) while using the reqctx instead of the tfm.
cryptd is used when simd drivers are used. cryptd_skcipher_encrypt() internally doesn't allocate a request ctx of a child, instead, it uses stack memory with SYNC_SKCIPHER_REQUEST_ON_STACK. It retains only 384 bytes for child request ctx even if a child set a large reqsize value with crypto_skcipher_set_reqsize().
aria-avx2 needs 512 bytes and aria-avx512 needs 1024 bytes.
So, stack-out-of-bounds occurs.

Quoted 9 lines

OK this is not supposed to happen.
---8<---
cryptd is buggy as it tries to use sync_skcipher without going through the proper sync_skcipher interface. In fact it doesn't even need sync_skcipher since it's already a proper skcipher and can easily access the request context instead of using something off the stack.

I have tested this patch with ctr(aria-avx), ctr(aria-avx2), and ctr(aria-avx512), and it works well. stack-out-of-bounds issues have disappeared after this patch.
So, I will test more and I will send a v4 patch.

Quoted 98 lines

Fixes 36b3875a97b8 ("crypto: cryptd - Remove VLA usage of skcipher")

Signed-off-by Herbert Xu <[email protected]>

crypto/cryptd.c +18 ~~-16~~

index 668095eca0fa..ca3a40fc7da9 100644
@@ -68,11 +68,12 @@ struct aead_instance_ctx {
 
  struct cryptd_skcipher_ctx {
  	refcount_t refcnt;
-	struct crypto_sync_skcipher *child;
+	struct crypto_skcipher *child;
  };
 
  struct cryptd_skcipher_request_ctx {
  	crypto_completion_t complete;
+	struct skcipher_request req;
  };
 
  struct cryptd_hash_ctx {

@@ -227,13 +228,13 @@ static int cryptd_skcipher_setkey(struct crypto_skcipher *parent,
  				  const u8 *key, unsigned int keylen)
  {
  	struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(parent);
-	struct crypto_sync_skcipher *child = ctx->child;
+	struct crypto_skcipher *child = ctx->child;
 
-	crypto_sync_skcipher_clear_flags(child, CRYPTO_TFM_REQ_MASK);
-	crypto_sync_skcipher_set_flags(child,
-				       crypto_skcipher_get_flags(parent) &
-					 CRYPTO_TFM_REQ_MASK);
-	return crypto_sync_skcipher_setkey(child, key, keylen);
+	crypto_skcipher_clear_flags(child, CRYPTO_TFM_REQ_MASK);
+	crypto_skcipher_set_flags(child,
+				  crypto_skcipher_get_flags(parent) &
+				  CRYPTO_TFM_REQ_MASK);
+	return crypto_skcipher_setkey(child, key, keylen);
  }
 
  static void cryptd_skcipher_complete(struct skcipher_request *req, int err)

@@ -258,13 +259,13 @@ static void cryptd_skcipher_encrypt(struct crypto_async_request *base,
  	struct cryptd_skcipher_request_ctx *rctx = skcipher_request_ctx(req);
  	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
  	struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
-	struct crypto_sync_skcipher *child = ctx->child;
-	SYNC_SKCIPHER_REQUEST_ON_STACK(subreq, child);
+	struct skcipher_request *subreq = &rctx->req;
+	struct crypto_skcipher *child = ctx->child;
 
  	if (unlikely(err == -EINPROGRESS))
  		goto out;
 
-	skcipher_request_set_sync_tfm(subreq, child);
+	skcipher_request_set_tfm(subreq, child);
  	skcipher_request_set_callback(subreq, CRYPTO_TFM_REQ_MAY_SLEEP,
  				      NULL, NULL);
  	skcipher_request_set_crypt(subreq, req->src, req->dst, req->cryptlen,

@@ -286,13 +287,13 @@ static void cryptd_skcipher_decrypt(struct crypto_async_request *base,
  	struct cryptd_skcipher_request_ctx *rctx = skcipher_request_ctx(req);
  	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
  	struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
-	struct crypto_sync_skcipher *child = ctx->child;
-	SYNC_SKCIPHER_REQUEST_ON_STACK(subreq, child);
+	struct skcipher_request *subreq = &rctx->req;
+	struct crypto_skcipher *child = ctx->child;
 
  	if (unlikely(err == -EINPROGRESS))
  		goto out;
 
-	skcipher_request_set_sync_tfm(subreq, child);
+	skcipher_request_set_tfm(subreq, child);
  	skcipher_request_set_callback(subreq, CRYPTO_TFM_REQ_MAY_SLEEP,
  				      NULL, NULL);
  	skcipher_request_set_crypt(subreq, req->src, req->dst, req->cryptlen,

@@ -343,9 +344,10 @@ static int cryptd_skcipher_init_tfm(struct crypto_skcipher *tfm)
  	if (IS_ERR(cipher))
  		return PTR_ERR(cipher);
 
-	ctx->child = (struct crypto_sync_skcipher *)cipher;
+	ctx->child = cipher;
  	crypto_skcipher_set_reqsize(
-		tfm, sizeof(struct cryptd_skcipher_request_ctx));
+		tfm, sizeof(struct cryptd_skcipher_request_ctx) +
+		     crypto_skcipher_reqsize(cipher));
  	return 0;
  }

@@ -353,7 +355,7 @@ static void cryptd_skcipher_exit_tfm(struct crypto_skcipher *tfm)
  {
  	struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
 
-	crypto_free_sync_skcipher(ctx->child);
+	crypto_free_skcipher(ctx->child);
  }
 
  static void cryptd_skcipher_free(struct skcipher_instance *inst)

crypto/cryptd.c struct crypto_skcipher *cryptd_skcipher_child(struct cryptd_skcipher *tfm) · line 939

Quoted 8 lines

crypto/cryptd.c +1 -1

@@ -931,7 +933,7 @@ struct crypto_skcipher *cryptd_skcipher_child(struct cryptd_skcipher *tfm)
  {
  	struct cryptd_skcipher_ctx *ctx = crypto_skcipher_ctx(&tfm->base);
 
-	return &ctx->child->base;
+	return ctx->child;
  }
  EXPORT_SYMBOL_GPL(cryptd_skcipher_child);

Thanks a lot,
Taehee Yoo

Taehee Yoo

author

2022-11-06 14:37 UTC #9 lore raw

PATCH v3 2/4 crypto: aria: do not use magic number offsets of aria_ctx

In reply to Taehee Yoo

aria-avx assembly code accesses members of aria_ctx with magic number offset. If the shape of struct aria_ctx is changed carelessly, aria-avx will not work.
So, we need to ensure accessing members of aria_ctx with correct offset values, not with magic numbers.

It adds ARIA_CTX_enc_key, ARIA_CTX_dec_key, and ARIA_CTX_rounds in the asm-offsets.c So, correct offset definitions will be generated. aria-avx assembly code can access members of aria_ctx safely with these definitions.

Signed-off-by Taehee Yoo <[email protected]>

---

v3:
- Patch introduced.

 arch/x86/crypto/aria-aesni-avx-asm_64.S | 26 +++++++++++--------------
 arch/x86/kernel/asm-offsets.c           | 11 +++++++++++
 2 files changed, 22 insertions(+), 15 deletions(-)

arch/x86/crypto/aria-aesni-avx-asm_64.S +11 ~~-15~~

index c75fd7d015ed..e47e7e54e08f 100644
@@ -8,11 +8,7 @@
 
 #include <linux/linkage.h>
 #include <asm/frame.h>
-
-/* struct aria_ctx: */
-#define enc_key 0
-#define dec_key 272
-#define rounds 544
+#include <asm/asm-offsets.h>
 
 /* register macros */
 #define CTX %rdi

@@ -873,7 +869,7 @@ SYM_FUNC_START_LOCAL(__aria_aesni_avx_crypt_16way)
 	aria_fo(%xmm9, %xmm8, %xmm11, %xmm10, %xmm12, %xmm13, %xmm14, %xmm15,
 		%xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
 		%rax, %r9, 10);
-	cmpl $12, rounds(CTX);
+	cmpl $12, ARIA_CTX_rounds(CTX);
 	jne .Laria_192;
 	aria_ff(%xmm1, %xmm0, %xmm3, %xmm2, %xmm4, %xmm5, %xmm6, %xmm7,
 		%xmm8, %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14,

@@ -886,7 +882,7 @@ SYM_FUNC_START_LOCAL(__aria_aesni_avx_crypt_16way)
 	aria_fo(%xmm9, %xmm8, %xmm11, %xmm10, %xmm12, %xmm13, %xmm14, %xmm15,
 		%xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
 		%rax, %r9, 12);
-	cmpl $14, rounds(CTX);
+	cmpl $14, ARIA_CTX_rounds(CTX);
 	jne .Laria_256;
 	aria_ff(%xmm1, %xmm0, %xmm3, %xmm2, %xmm4, %xmm5, %xmm6, %xmm7,
 		%xmm8, %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14,

@@ -922,7 +918,7 @@ SYM_FUNC_START(aria_aesni_avx_encrypt_16way)
 
 	FRAME_BEGIN
 
-	leaq enc_key(CTX), %r9;
+	leaq ARIA_CTX_enc_key(CTX), %r9;
 
 	inpack16_pre(%xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
 		     %xmm8, %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14,

@@ -947,7 +943,7 @@ SYM_FUNC_START(aria_aesni_avx_decrypt_16way)
 
 	FRAME_BEGIN
 
-	leaq dec_key(CTX), %r9;
+	leaq ARIA_CTX_dec_key(CTX), %r9;
 
 	inpack16_pre(%xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
 		     %xmm8, %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14,

@@ -1055,7 +1051,7 @@ SYM_FUNC_START(aria_aesni_avx_ctr_crypt_16way)
 	leaq (%rdx), %r11;
 	leaq (%rcx), %rsi;
 	leaq (%rcx), %rdx;
-	leaq enc_key(CTX), %r9;
+	leaq ARIA_CTX_enc_key(CTX), %r9;
 
 	call __aria_aesni_avx_crypt_16way;

@@ -1156,7 +1152,7 @@ SYM_FUNC_START_LOCAL(__aria_aesni_avx_gfni_crypt_16way)
 		     %xmm0, %xmm1, %xmm2, %xmm3,
 		     %xmm4, %xmm5, %xmm6, %xmm7,
 		     %rax, %r9, 10);
-	cmpl $12, rounds(CTX);
+	cmpl $12, ARIA_CTX_rounds(CTX);
 	jne .Laria_gfni_192;
 	aria_ff_gfni(%xmm1, %xmm0, %xmm3, %xmm2, %xmm4, %xmm5, %xmm6, %xmm7,
 		%xmm8, %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14,

@@ -1173,7 +1169,7 @@ SYM_FUNC_START_LOCAL(__aria_aesni_avx_gfni_crypt_16way)
 		     %xmm0, %xmm1, %xmm2, %xmm3,
 		     %xmm4, %xmm5, %xmm6, %xmm7,
 		     %rax, %r9, 12);
-	cmpl $14, rounds(CTX);
+	cmpl $14, ARIA_CTX_rounds(CTX);
 	jne .Laria_gfni_256;
 	aria_ff_gfni(%xmm1, %xmm0, %xmm3, %xmm2,
 		     %xmm4, %xmm5, %xmm6, %xmm7,

@@ -1217,7 +1213,7 @@ SYM_FUNC_START(aria_aesni_avx_gfni_encrypt_16way)
 
 	FRAME_BEGIN
 
-	leaq enc_key(CTX), %r9;
+	leaq ARIA_CTX_enc_key(CTX), %r9;
 
 	inpack16_pre(%xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
 		     %xmm8, %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14,

@@ -1242,7 +1238,7 @@ SYM_FUNC_START(aria_aesni_avx_gfni_decrypt_16way)
 
 	FRAME_BEGIN
 
-	leaq dec_key(CTX), %r9;
+	leaq ARIA_CTX_dec_key(CTX), %r9;
 
 	inpack16_pre(%xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
 		     %xmm8, %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14,

@@ -1274,7 +1270,7 @@ SYM_FUNC_START(aria_aesni_avx_gfni_ctr_crypt_16way)
 	leaq (%rdx), %r11;
 	leaq (%rcx), %rsi;
 	leaq (%rcx), %rdx;
-	leaq enc_key(CTX), %r9;
+	leaq ARIA_CTX_enc_key(CTX), %r9;
 
 	call __aria_aesni_avx_gfni_crypt_16way;

arch/x86/kernel/asm-offsets.c +11 -0

index cb50589a7102..32192a91c65b 100644
@@ -7,6 +7,7 @@
 #define COMPILE_OFFSETS
 
 #include <linux/crypto.h>
+#include <crypto/aria.h>
 #include <linux/sched.h>
 #include <linux/stddef.h>
 #include <linux/hardirq.h>

@@ -109,6 +110,16 @@ static void __used common(void)
 	OFFSET(TSS_sp1, tss_struct, x86_tss.sp1);
 	OFFSET(TSS_sp2, tss_struct, x86_tss.sp2);
 
+#if defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64) ||  \
+	defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64_MODULE)
+
+	/* Offset for fields in aria_ctx */
+	BLANK();
+	OFFSET(ARIA_CTX_enc_key, aria_ctx, enc_key);
+	OFFSET(ARIA_CTX_dec_key, aria_ctx, dec_key);
+	OFFSET(ARIA_CTX_rounds, aria_ctx, rounds);
+#endif
+
 	if (IS_ENABLED(CONFIG_KVM_INTEL)) {
 		BLANK();
 		OFFSET(VMX_spec_ctrl, vcpu_vmx, spec_ctrl);

Signature · 1 line

2.17.1

Elliott, Robert (Servers)

2022-11-10 03:57 UTC #10 lore raw

RE: [PATCH v3 2/4] crypto: aria: do not use magic number offsets of aria_ctx

Quoted 27 lines

-----Original Message-----
From: Taehee Yoo <[email protected]>
Sent: Sunday, November 6, 2022 8:36 AM
To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Elliott, Robert (Servers) <[email protected]>; [email protected]; [email protected]
Cc: [email protected]
Subject: [PATCH v3 2/4] crypto: aria: do not use magic number offsets of aria_ctx
aria-avx assembly code accesses members of aria_ctx with magic number offset. If the shape of struct aria_ctx is changed carelessly, aria-avx will not work.
So, we need to ensure accessing members of aria_ctx with correct offset values, not with magic numbers.
It adds ARIA_CTX_enc_key, ARIA_CTX_dec_key, and ARIA_CTX_rounds in the asm-offsets.c So, correct offset definitions will be generated. aria-avx assembly code can access members of aria_ctx safely with these definitions.
(quoted diff) +0 -0

...

arch/x86/crypto/aria-aesni-avx-asm_64.S line 15

Quoted 8 lines

#include <linux/linkage.h>
#include <asm/frame.h>
arch/x86/crypto/aria-aesni-avx-asm_64.S +0 -5
-
-/* struct aria_ctx: */
-#define enc_key 0
-#define dec_key 272
-#define rounds 544

That structure also has a key_length field after the rounds field. aria_set_key() sets it, but no function ever seems to use it. Perhaps that field should be removed?

Quoted 1 line

+#include <asm/asm-offsets.h>

That makes the offsets flexible, which is good.

The assembly code also implicitly assumes the size of each of those fields (e.g., enc_key is 272 bytes, dec_key is 272 bytes, and rounds is 4 bytes). A BUILD_BUG_ON confirming those assumptions might be worthwhile.

arch/x86/kernel/asm-offsets.c line 10

Quoted 9 lines

arch/x86/kernel/asm-offsets.c +1 -0

index cb50589a7102..32192a91c65b 100644
@@ -7,6 +7,7 @@
 #define COMPILE_OFFSETS
 
 #include <linux/crypto.h>
+#include <crypto/aria.h>

Is it safe to include .h files for a large number of crypto modules in one C file? It seems like they could easily include naming conflicts.

However, this set does seem to compile cleanly:

+// no .h for aegis, but ctx is simple
+#include <crypto/aes.h>
+#include <crypto/aria.h>
+#include <crypto/blake2s.h>
+#include <crypto/blowfish.h>
+// no .h for camellia in crypto; it is in arch/x86/crypto +#include <crypto/cast5.h>
+#include <crypto/cast6.h>
+#include <crypto/chacha.h>
+// no .h for crc32c, crc32, crct10dif, but no ctx structure to worry about +#include <crypto/curve25519.h>
+#include <crypto/des.h>
+#include <crypto/ghash.h>
+#include <crypto/nhpoly1305.h>
+#include <crypto/poly1305.h>
+#include <crypto/polyval.h>
+#include <crypto/serpent.h>
+#include <crypto/sha1.h>
+#include <crypto/sha2.h>
+#include <crypto/sm3.h>
+#include <crypto/twofish.h>

Taehee Yoo

author

2022-11-11 06:45 UTC #11 lore raw

Re: [PATCH v3 2/4] crypto: aria: do not use magic number offsets of aria_ctx

Hi Elliott,
Thank you so much for your review!

Quoted 29 lines · Robert (Servers)

Quoted 27 lines
-----Original Message-----
From: Taehee Yoo <[email protected]>
Sent: Sunday, November 6, 2022 8:36 AM
To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Elliott, Robert (Servers) <[email protected]>; [email protected]; [email protected]
Cc: [email protected]
Subject: [PATCH v3 2/4] crypto: aria: do not use magic number offsets of aria_ctx
aria-avx assembly code accesses members of aria_ctx with magic number offset. If the shape of struct aria_ctx is changed carelessly, aria-avx will not work.
So, we need to ensure accessing members of aria_ctx with correct offset values, not with magic numbers.
It adds ARIA_CTX_enc_key, ARIA_CTX_dec_key, and ARIA_CTX_rounds in the asm-offsets.c So, correct offset definitions will be generated. aria-avx assembly code can access members of aria_ctx safely with these definitions.
(quoted diff) +0 -0
...
Quoted 1 line

Quoted 11 lines

arch/x86/crypto/aria-aesni-avx-asm_64.S line 15
Quoted 7 lines
  #include <linux/linkage.h>
  #include <asm/frame.h>
arch/x86/crypto/aria-aesni-avx-asm_64.S +0 -5
-
-/* struct aria_ctx: */
-#define enc_key 0
-#define dec_key 272
-#define rounds 544
That structure also has a key_length field after the rounds field. aria_set_key() sets it, but no function ever seems to use it. Perhaps that field should be removed?

Okay, I will remove that fields in a separate patch.

Quoted 8 lines

Quoted 2 lines
+#include <asm/asm-offsets.h>
That makes the offsets flexible, which is good.
The assembly code also implicitly assumes the size of each of those fields (e.g., enc_key is 272 bytes, dec_key is 272 bytes, and rounds is 4 bytes). A BUILD_BUG_ON confirming those assumptions might be worthwhile.

You're right,
I didn't consider the size of the fields.
I will contain BUILD_BUG_ON() to check the size.

Quoted 8 lines

Quoted 8 lines
arch/x86/kernel/asm-offsets.c +0 -0
index cb50589a7102..32192a91c65b 100644
@@ -7,6 +7,7 @@
  #define COMPILE_OFFSETS
 

Quoted 8 lines

arch/x86/kernel/asm-offsets.c line 10
Quoted 2 lines
arch/x86/kernel/asm-offsets.c +1 -0
  #include <linux/crypto.h>
+#include <crypto/aria.h>
Is it safe to include .h files for a large number of crypto modules in one C file? It seems like they could easily include naming conflicts.
However, this set does seem to compile cleanly:

Thanks for this check.
Sorry, I'm not sure about the side effects of a large number of header in .c file.

Quoted 12 lines

(quoted diff) +11 -0

+// no .h for aegis, but ctx is simple
+#include <crypto/aes.h>
+#include <crypto/aria.h>
+#include <crypto/blake2s.h>
+#include <crypto/blowfish.h>
+// no .h for camellia in crypto; it is in arch/x86/crypto
+#include <crypto/cast5.h>
+#include <crypto/cast6.h>
+#include <crypto/chacha.h>
+// no .h for crc32c, crc32, crct10dif, but no ctx structure to worry about
+#include <crypto/curve25519.h>

Review

Quoted 10 lines

(quoted diff) +10 -0

+#include <crypto/des.h>
+#include <crypto/ghash.h>
+#include <crypto/nhpoly1305.h>
+#include <crypto/poly1305.h>
+#include <crypto/polyval.h>
+#include <crypto/serpent.h>
+#include <crypto/sha1.h>
+#include <crypto/sha2.h>
+#include <crypto/sm3.h>
+#include <crypto/twofish.h>

Thanks a lot!
Taehee Yoo

Taehee Yoo

author

2022-11-06 14:37 UTC #12 lore raw

PATCH v3 3/4 crypto: aria: implement aria-avx2

In reply to Taehee Yoo

aria-avx2 implementation uses AVX2, AES-NI, and GFNI.
It supports 32way parallel processing.
So, byteslicing code is changed to support 32way parallel. And it exports some aria-avx functions such as encrypt() and decrypt().

There are two main logics, s-box layer and diffusion layer. These codes are the same as aria-avx implementation.
But some instruction are exchanged because they don't support 256bit registers.
Also, AES-NI doesn't support 256bit register.
So, aesenclast and aesdeclast are used twice like below:

	vextracti128 $1, ymm0, xmm6;
	vaesenclast xmm7, xmm0, xmm0;
	vaesenclast xmm7, xmm6, xmm6;
	vinserti128 $1, xmm6, ymm0, ymm0;

Benchmark with modprobe tcrypt mode=610 num_mb=8192, i3-12100:

Signed-off-by Taehee Yoo <[email protected]>

---

v3:
- Use ARIA_CTX_enc_key, ARIA_CTX_dec_key, and ARIA_CTX_rounds defines.

v2:
- Add new "add keystream array into struct aria_ctx" patch.
- Use keystream array in the aria_ctx instead of stack memory

 arch/x86/crypto/Kconfig                  |   19 +
 arch/x86/crypto/Makefile                 |    3 +
 arch/x86/crypto/aria-aesni-avx2-asm_64.S | 1432 ++++++++++++++++++++++
 arch/x86/crypto/aria-avx.h               |   35 +
 arch/x86/crypto/aria_aesni_avx2_glue.c   |  236 ++++
 arch/x86/crypto/aria_aesni_avx_glue.c    |    6 +
 include/crypto/aria.h                    |    9 +-
 7 files changed, 1739 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/crypto/aria-aesni-avx2-asm_64.S
 create mode 100644 arch/x86/crypto/aria_aesni_avx2_glue.c

arch/x86/crypto/Kconfig +19 -0

index 71c4c473d34b..3837ba8b78c5 100644
@@ -304,6 +304,25 @@ config CRYPTO_ARIA_AESNI_AVX_X86_64
 
 	  Processes 16 blocks in parallel.
 
+config CRYPTO_ARIA_AESNI_AVX2_X86_64
+	tristate "Ciphers: ARIA with modes: ECB, CTR (AES-NI/AVX2/GFNI)"
+	depends on X86 && 64BIT
+	select CRYPTO_SKCIPHER
+	select CRYPTO_SIMD
+	select CRYPTO_ALGAPI
+	select CRYPTO_ARIA
+	select CRYPTO_ARIA_AESNI_AVX_X86_64
+	help
+	  Length-preserving cipher: ARIA cipher algorithms
+	  (RFC 5794) with ECB and CTR modes
+
+	  Architecture: x86_64 using:
+	  - AES-NI (AES New Instructions)
+	  - AVX2 (Advanced Vector Extensions)
+	  - GFNI (Galois Field New Instructions)
+
+	  Processes 32 blocks in parallel.
+
 config CRYPTO_CHACHA20_X86_64
 	tristate "Ciphers: ChaCha20, XChaCha20, XChaCha12 (SSSE3/AVX2/AVX-512VL)"
 	depends on X86 && 64BIT

arch/x86/crypto/Makefile +3 -0

index 3b1d701a4f6c..5a349c7a8127 100644
@@ -103,6 +103,9 @@ sm4-aesni-avx2-x86_64-y := sm4-aesni-avx2-asm_64.o sm4_aesni_avx2_glue.o
 obj-$(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64) += aria-aesni-avx-x86_64.o
 aria-aesni-avx-x86_64-y := aria-aesni-avx-asm_64.o aria_aesni_avx_glue.o
 
+obj-$(CONFIG_CRYPTO_ARIA_AESNI_AVX2_X86_64) += aria-aesni-avx2-x86_64.o
+aria-aesni-avx2-x86_64-y := aria-aesni-avx2-asm_64.o aria_aesni_avx2_glue.o
+
 quiet_cmd_perlasm = PERLASM $@
       cmd_perlasm = $(PERL) $< > $@
 $(obj)/%.S: $(src)/%.pl FORCE

arch/x86/crypto/aria-aesni-avx2-asm_64.S new file +1432 -0

1,432 lines not on this page · raw message

arch/x86/crypto/aria-avx.h +35 -0

index afd37af95e58..5dd7f18255eb 100644
@@ -4,10 +4,45 @@
 
 #include <linux/types.h>
 
+asmlinkage void aria_aesni_avx_encrypt_16way(const void *ctx, u8 *dst,
+					     const u8 *src);
+asmlinkage void aria_aesni_avx_decrypt_16way(const void *ctx, u8 *dst,
+					     const u8 *src);
+asmlinkage void aria_aesni_avx_ctr_crypt_16way(const void *ctx, u8 *dst,
+					       const u8 *src,
+					       u8 *keystream, u8 *iv);
+asmlinkage void aria_aesni_avx_gfni_encrypt_16way(const void *ctx, u8 *dst,
+						  const u8 *src);
+asmlinkage void aria_aesni_avx_gfni_decrypt_16way(const void *ctx, u8 *dst,
+						  const u8 *src);
+asmlinkage void aria_aesni_avx_gfni_ctr_crypt_16way(const void *ctx, u8 *dst,
+						    const u8 *src,
+						    u8 *keystream, u8 *iv);
+
+asmlinkage void aria_aesni_avx2_encrypt_32way(const void *ctx, u8 *dst,
+					      const u8 *src);
+asmlinkage void aria_aesni_avx2_decrypt_32way(const void *ctx, u8 *dst,
+					      const u8 *src);
+asmlinkage void aria_aesni_avx2_ctr_crypt_32way(const void *ctx, u8 *dst,
+						const u8 *src,
+						u8 *keystream, u8 *iv);
+asmlinkage void aria_aesni_avx2_gfni_encrypt_32way(const void *ctx, u8 *dst,
+						   const u8 *src);
+asmlinkage void aria_aesni_avx2_gfni_decrypt_32way(const void *ctx, u8 *dst,
+						   const u8 *src);
+asmlinkage void aria_aesni_avx2_gfni_ctr_crypt_32way(const void *ctx, u8 *dst,
+						     const u8 *src,
+						     u8 *keystream, u8 *iv);
+
 struct aria_avx_ops {
 	void (*aria_encrypt_16way)(const void *ctx, u8 *dst, const u8 *src);
 	void (*aria_decrypt_16way)(const void *ctx, u8 *dst, const u8 *src);
 	void (*aria_ctr_crypt_16way)(const void *ctx, u8 *dst, const u8 *src,
 				     u8 *keystream, u8 *iv);
+	void (*aria_encrypt_32way)(const void *ctx, u8 *dst, const u8 *src);
+	void (*aria_decrypt_32way)(const void *ctx, u8 *dst, const u8 *src);
+	void (*aria_ctr_crypt_32way)(const void *ctx, u8 *dst, const u8 *src,
+				     u8 *keystream, u8 *iv);
+
 };
 #endif

arch/x86/crypto/aria_aesni_avx2_glue.c new file +236 -0

236 lines not on this page · raw message

arch/x86/crypto/aria_aesni_avx_glue.c +6 -0

index b122482d0c9d..9999b7d2deac 100644
@@ -18,18 +18,24 @@
 
 asmlinkage void aria_aesni_avx_encrypt_16way(const void *ctx, u8 *dst,
 					     const u8 *src);
+EXPORT_SYMBOL_GPL(aria_aesni_avx_encrypt_16way);
 asmlinkage void aria_aesni_avx_decrypt_16way(const void *ctx, u8 *dst,
 					     const u8 *src);
+EXPORT_SYMBOL_GPL(aria_aesni_avx_decrypt_16way);
 asmlinkage void aria_aesni_avx_ctr_crypt_16way(const void *ctx, u8 *dst,
 					       const u8 *src,
 					       u8 *keystream, u8 *iv);
+EXPORT_SYMBOL_GPL(aria_aesni_avx_ctr_crypt_16way);
 asmlinkage void aria_aesni_avx_gfni_encrypt_16way(const void *ctx, u8 *dst,
 						  const u8 *src);
+EXPORT_SYMBOL_GPL(aria_aesni_avx_gfni_encrypt_16way);
 asmlinkage void aria_aesni_avx_gfni_decrypt_16way(const void *ctx, u8 *dst,
 						  const u8 *src);
+EXPORT_SYMBOL_GPL(aria_aesni_avx_gfni_decrypt_16way);
 asmlinkage void aria_aesni_avx_gfni_ctr_crypt_16way(const void *ctx, u8 *dst,
 						    const u8 *src,
 						    u8 *keystream, u8 *iv);
+EXPORT_SYMBOL_GPL(aria_aesni_avx_gfni_ctr_crypt_16way);
 
 static struct aria_avx_ops aria_ops;

include/crypto/aria.h +8 -1

index f5c7a87378cd..bc5dc71d6a98 100644
@@ -33,7 +33,14 @@
 
 #define ARIA_AESNI_PARALLEL_BLOCKS 16
 #define ARIA_AESNI_PARALLEL_BLOCK_SIZE  (ARIA_BLOCK_SIZE * 16)
-#if defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64) ||	\
+
+#define ARIA_AESNI_AVX2_PARALLEL_BLOCKS 32
+#define ARIA_AESNI_AVX2_PARALLEL_BLOCK_SIZE  (ARIA_BLOCK_SIZE * 32)
+
+#if defined(CONFIG_CRYPTO_ARIA_AESNI_AVX2_X86_64) ||   \
+	defined(CONFIG_CRYPTO_ARIA_AESNI_AVX2_X86_64_MODULE)
+#define ARIA_KEYSTREAM_SIZE ARIA_AESNI_AVX2_PARALLEL_BLOCK_SIZE
+#elif defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64) ||  \
 	defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64_MODULE)
 #define ARIA_KEYSTREAM_SIZE ARIA_AESNI_PARALLEL_BLOCK_SIZE
 #endif

Signature · 1 line

2.17.1

Taehee Yoo

author

2022-11-06 14:38 UTC #13 lore raw

PATCH v3 4/4 crypto: aria: implement aria-avx512

In reply to Taehee Yoo

aria-avx512 implementation uses AVX512 and GFNI.
It supports 64way parallel processing.
So, byteslicing code is changed to support 64way parallel. And it exports some aria-avx2 functions such as encrypt() and decrypt().

AVX and AVX2 have 16 registers.
They should use memory to store/load state because of lack of registers. But AVX512 supports 32 registers.
So, it doesn't require store/load in the s-box layer.
It means that it can reduce overhead of store/load in the s-box layer. Also code become much simpler.

Benchmark with modprobe tcrypt mode=610 num_mb=8192, i3-12100:

Signed-off-by Taehee Yoo <[email protected]>

---

v3:
- Use ARIA_CTX_enc_key, ARIA_CTX_dec_key, and ARIA_CTX_rounds defines.

v2:
- Add new "add keystream array into struct aria_ctx" patch.
- Use keystream array in the aria_ctx instead of stack memory

 arch/x86/crypto/Kconfig                   |   19 +
 arch/x86/crypto/Makefile                  |    3 +
 arch/x86/crypto/aria-avx.h                |    5 +
 arch/x86/crypto/aria-gfni-avx512-asm_64.S | 1019 +++++++++++++++++++++
 arch/x86/crypto/aria_gfni_avx512_glue.c   |  232 +++++
 include/crypto/aria.h                     |    8 +-
 6 files changed, 1285 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/crypto/aria-gfni-avx512-asm_64.S
 create mode 100644 arch/x86/crypto/aria_gfni_avx512_glue.c

arch/x86/crypto/Kconfig +19 -0

index 3837ba8b78c5..688e848f740d 100644
@@ -323,6 +323,25 @@ config CRYPTO_ARIA_AESNI_AVX2_X86_64
 
 	  Processes 32 blocks in parallel.
 
+config CRYPTO_ARIA_GFNI_AVX512_X86_64
+	tristate "Ciphers: ARIA with modes: ECB, CTR (AVX512/GFNI)"
+	depends on X86 && 64BIT
+	select CRYPTO_SKCIPHER
+	select CRYPTO_SIMD
+	select CRYPTO_ALGAPI
+	select CRYPTO_ARIA
+	select CRYPTO_ARIA_AESNI_AVX_X86_64
+	select CRYPTO_ARIA_AESNI_AVX2_X86_64
+	help
+	  Length-preserving cipher: ARIA cipher algorithms
+	  (RFC 5794) with ECB and CTR modes
+
+	  Architecture: x86_64 using:
+	  - AVX512 (Advanced Vector Extensions)
+	  - GFNI (Galois Field New Instructions)
+
+	  Processes 64 blocks in parallel.
+
 config CRYPTO_CHACHA20_X86_64
 	tristate "Ciphers: ChaCha20, XChaCha20, XChaCha12 (SSSE3/AVX2/AVX-512VL)"
 	depends on X86 && 64BIT

arch/x86/crypto/Makefile +3 -0

index 5a349c7a8127..1844925f1f4a 100644
@@ -106,6 +106,9 @@ aria-aesni-avx-x86_64-y := aria-aesni-avx-asm_64.o aria_aesni_avx_glue.o
 obj-$(CONFIG_CRYPTO_ARIA_AESNI_AVX2_X86_64) += aria-aesni-avx2-x86_64.o
 aria-aesni-avx2-x86_64-y := aria-aesni-avx2-asm_64.o aria_aesni_avx2_glue.o
 
+obj-$(CONFIG_CRYPTO_ARIA_GFNI_AVX512_X86_64) += aria-gfni-avx512-x86_64.o
+aria-gfni-avx512-x86_64-y := aria-gfni-avx512-asm_64.o aria_gfni_avx512_glue.o
+
 quiet_cmd_perlasm = PERLASM $@
       cmd_perlasm = $(PERL) $< > $@
 $(obj)/%.S: $(src)/%.pl FORCE

arch/x86/crypto/aria-avx.h +5 -0

index 5dd7f18255eb..b62c255ce850 100644
@@ -43,6 +43,11 @@ struct aria_avx_ops {
 	void (*aria_decrypt_32way)(const void *ctx, u8 *dst, const u8 *src);
 	void (*aria_ctr_crypt_32way)(const void *ctx, u8 *dst, const u8 *src,
 				     u8 *keystream, u8 *iv);
+	void (*aria_encrypt_64way)(const void *ctx, u8 *dst, const u8 *src);
+	void (*aria_decrypt_64way)(const void *ctx, u8 *dst, const u8 *src);
+	void (*aria_ctr_crypt_64way)(const void *ctx, u8 *dst, const u8 *src,
+				     u8 *keystream, u8 *iv);
+
 
 };
 #endif

arch/x86/crypto/aria-gfni-avx512-asm_64.S new file +1019 -0

1,019 lines not on this page · raw message

arch/x86/crypto/aria_gfni_avx512_glue.c new file +232 -0

232 lines not on this page · raw message

include/crypto/aria.h +7 -1

index bc5dc71d6a98..50164cdf996b 100644
@@ -37,7 +37,13 @@
 #define ARIA_AESNI_AVX2_PARALLEL_BLOCKS 32
 #define ARIA_AESNI_AVX2_PARALLEL_BLOCK_SIZE  (ARIA_BLOCK_SIZE * 32)
 
-#if defined(CONFIG_CRYPTO_ARIA_AESNI_AVX2_X86_64) ||   \
+#define ARIA_GFNI_AVX512_PARALLEL_BLOCKS 64
+#define ARIA_GFNI_AVX512_PARALLEL_BLOCK_SIZE  (ARIA_BLOCK_SIZE * 64)
+
+#if defined(CRYPTO_ARIA_GFNI_AVX512_X86_64) || \
+	defined(CRYPTO_ARIA_GFNI_AVX512_X86_64_MODULE)
+#define ARIA_KEYSTREAM_SIZE ARIA_GFNI_AVX512_PARALLEL_BLOCK_SIZE
+#elif defined(CONFIG_CRYPTO_ARIA_AESNI_AVX2_X86_64) ||   \
 	defined(CONFIG_CRYPTO_ARIA_AESNI_AVX2_X86_64_MODULE)
 #define ARIA_KEYSTREAM_SIZE ARIA_AESNI_AVX2_PARALLEL_BLOCK_SIZE
 #elif defined(CONFIG_CRYPTO_ARIA_AESNI_AVX_X86_64) ||  \

Signature · 1 line

2.17.1