All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] crypto: padlock-aes - convert to skcipher API
@ 2019-10-13  4:17 Eric Biggers
  2019-10-13 23:20 ` Jamie Heilman
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Eric Biggers @ 2019-10-13  4:17 UTC (permalink / raw)
  To: linux-crypto, Herbert Xu; +Cc: Jamie Heilman

From: Eric Biggers <ebiggers@google.com>

Convert the VIA PadLock implementations of AES-ECB and AES-CBC from the
deprecated "blkcipher" API to the "skcipher" API.  This is needed in
order for the blkcipher API to be removed.

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

This is compile-tested only, as I don't have this hardware.
If anyone has this hardware, please test it with
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y.

 drivers/crypto/padlock-aes.c | 157 +++++++++++++++++------------------
 1 file changed, 74 insertions(+), 83 deletions(-)

diff --git a/drivers/crypto/padlock-aes.c b/drivers/crypto/padlock-aes.c
index 8a0661250078..c5b60f50e1b5 100644
--- a/drivers/crypto/padlock-aes.c
+++ b/drivers/crypto/padlock-aes.c
@@ -10,6 +10,7 @@
 
 #include <crypto/algapi.h>
 #include <crypto/aes.h>
+#include <crypto/internal/skcipher.h>
 #include <crypto/padlock.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -97,9 +98,9 @@ static inline struct aes_ctx *aes_ctx(struct crypto_tfm *tfm)
 	return aes_ctx_common(crypto_tfm_ctx(tfm));
 }
 
-static inline struct aes_ctx *blk_aes_ctx(struct crypto_blkcipher *tfm)
+static inline struct aes_ctx *skcipher_aes_ctx(struct crypto_skcipher *tfm)
 {
-	return aes_ctx_common(crypto_blkcipher_ctx(tfm));
+	return aes_ctx_common(crypto_skcipher_ctx(tfm));
 }
 
 static int aes_set_key(struct crypto_tfm *tfm, const u8 *in_key,
@@ -162,6 +163,12 @@ static int aes_set_key(struct crypto_tfm *tfm, const u8 *in_key,
 	return 0;
 }
 
+static int aes_set_key_skcipher(struct crypto_skcipher *tfm, const u8 *in_key,
+				unsigned int key_len)
+{
+	return aes_set_key(crypto_skcipher_tfm(tfm), in_key, key_len);
+}
+
 /* ====== Encryption/decryption routines ====== */
 
 /* These are the real call to PadLock. */
@@ -338,25 +345,24 @@ static struct crypto_alg aes_alg = {
 	}
 };
 
-static int ecb_aes_encrypt(struct blkcipher_desc *desc,
-			   struct scatterlist *dst, struct scatterlist *src,
-			   unsigned int nbytes)
+static int ecb_aes_encrypt(struct skcipher_request *req)
 {
-	struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
-	struct blkcipher_walk walk;
+	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+	struct aes_ctx *ctx = skcipher_aes_ctx(tfm);
+	struct skcipher_walk walk;
+	unsigned int nbytes;
 	int err;
 
 	padlock_reset_key(&ctx->cword.encrypt);
 
-	blkcipher_walk_init(&walk, dst, src, nbytes);
-	err = blkcipher_walk_virt(desc, &walk);
+	err = skcipher_walk_virt(&walk, req, false);
 
-	while ((nbytes = walk.nbytes)) {
+	while ((nbytes = walk.nbytes) != 0) {
 		padlock_xcrypt_ecb(walk.src.virt.addr, walk.dst.virt.addr,
 				   ctx->E, &ctx->cword.encrypt,
 				   nbytes / AES_BLOCK_SIZE);
 		nbytes &= AES_BLOCK_SIZE - 1;
-		err = blkcipher_walk_done(desc, &walk, nbytes);
+		err = skcipher_walk_done(&walk, nbytes);
 	}
 
 	padlock_store_cword(&ctx->cword.encrypt);
@@ -364,25 +370,24 @@ static int ecb_aes_encrypt(struct blkcipher_desc *desc,
 	return err;
 }
 
-static int ecb_aes_decrypt(struct blkcipher_desc *desc,
-			   struct scatterlist *dst, struct scatterlist *src,
-			   unsigned int nbytes)
+static int ecb_aes_decrypt(struct skcipher_request *req)
 {
-	struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
-	struct blkcipher_walk walk;
+	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+	struct aes_ctx *ctx = skcipher_aes_ctx(tfm);
+	struct skcipher_walk walk;
+	unsigned int nbytes;
 	int err;
 
 	padlock_reset_key(&ctx->cword.decrypt);
 
-	blkcipher_walk_init(&walk, dst, src, nbytes);
-	err = blkcipher_walk_virt(desc, &walk);
+	err = skcipher_walk_virt(&walk, req, false);
 
-	while ((nbytes = walk.nbytes)) {
+	while ((nbytes = walk.nbytes) != 0) {
 		padlock_xcrypt_ecb(walk.src.virt.addr, walk.dst.virt.addr,
 				   ctx->D, &ctx->cword.decrypt,
 				   nbytes / AES_BLOCK_SIZE);
 		nbytes &= AES_BLOCK_SIZE - 1;
-		err = blkcipher_walk_done(desc, &walk, nbytes);
+		err = skcipher_walk_done(&walk, nbytes);
 	}
 
 	padlock_store_cword(&ctx->cword.encrypt);
@@ -390,48 +395,41 @@ static int ecb_aes_decrypt(struct blkcipher_desc *desc,
 	return err;
 }
 
-static struct crypto_alg ecb_aes_alg = {
-	.cra_name		=	"ecb(aes)",
-	.cra_driver_name	=	"ecb-aes-padlock",
-	.cra_priority		=	PADLOCK_COMPOSITE_PRIORITY,
-	.cra_flags		=	CRYPTO_ALG_TYPE_BLKCIPHER,
-	.cra_blocksize		=	AES_BLOCK_SIZE,
-	.cra_ctxsize		=	sizeof(struct aes_ctx),
-	.cra_alignmask		=	PADLOCK_ALIGNMENT - 1,
-	.cra_type		=	&crypto_blkcipher_type,
-	.cra_module		=	THIS_MODULE,
-	.cra_u			=	{
-		.blkcipher = {
-			.min_keysize		=	AES_MIN_KEY_SIZE,
-			.max_keysize		=	AES_MAX_KEY_SIZE,
-			.setkey	   		= 	aes_set_key,
-			.encrypt		=	ecb_aes_encrypt,
-			.decrypt		=	ecb_aes_decrypt,
-		}
-	}
+static struct skcipher_alg ecb_aes_alg = {
+	.base.cra_name		=	"ecb(aes)",
+	.base.cra_driver_name	=	"ecb-aes-padlock",
+	.base.cra_priority	=	PADLOCK_COMPOSITE_PRIORITY,
+	.base.cra_blocksize	=	AES_BLOCK_SIZE,
+	.base.cra_ctxsize	=	sizeof(struct aes_ctx),
+	.base.cra_alignmask	=	PADLOCK_ALIGNMENT - 1,
+	.base.cra_module	=	THIS_MODULE,
+	.min_keysize		=	AES_MIN_KEY_SIZE,
+	.max_keysize		=	AES_MAX_KEY_SIZE,
+	.setkey			=	aes_set_key_skcipher,
+	.encrypt		=	ecb_aes_encrypt,
+	.decrypt		=	ecb_aes_decrypt,
 };
 
-static int cbc_aes_encrypt(struct blkcipher_desc *desc,
-			   struct scatterlist *dst, struct scatterlist *src,
-			   unsigned int nbytes)
+static int cbc_aes_encrypt(struct skcipher_request *req)
 {
-	struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
-	struct blkcipher_walk walk;
+	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+	struct aes_ctx *ctx = skcipher_aes_ctx(tfm);
+	struct skcipher_walk walk;
+	unsigned int nbytes;
 	int err;
 
 	padlock_reset_key(&ctx->cword.encrypt);
 
-	blkcipher_walk_init(&walk, dst, src, nbytes);
-	err = blkcipher_walk_virt(desc, &walk);
+	err = skcipher_walk_virt(&walk, req, false);
 
-	while ((nbytes = walk.nbytes)) {
+	while ((nbytes = walk.nbytes) != 0) {
 		u8 *iv = padlock_xcrypt_cbc(walk.src.virt.addr,
 					    walk.dst.virt.addr, ctx->E,
 					    walk.iv, &ctx->cword.encrypt,
 					    nbytes / AES_BLOCK_SIZE);
 		memcpy(walk.iv, iv, AES_BLOCK_SIZE);
 		nbytes &= AES_BLOCK_SIZE - 1;
-		err = blkcipher_walk_done(desc, &walk, nbytes);
+		err = skcipher_walk_done(&walk, nbytes);
 	}
 
 	padlock_store_cword(&ctx->cword.decrypt);
@@ -439,25 +437,24 @@ static int cbc_aes_encrypt(struct blkcipher_desc *desc,
 	return err;
 }
 
-static int cbc_aes_decrypt(struct blkcipher_desc *desc,
-			   struct scatterlist *dst, struct scatterlist *src,
-			   unsigned int nbytes)
+static int cbc_aes_decrypt(struct skcipher_request *req)
 {
-	struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
-	struct blkcipher_walk walk;
+	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+	struct aes_ctx *ctx = skcipher_aes_ctx(tfm);
+	struct skcipher_walk walk;
+	unsigned int nbytes;
 	int err;
 
 	padlock_reset_key(&ctx->cword.encrypt);
 
-	blkcipher_walk_init(&walk, dst, src, nbytes);
-	err = blkcipher_walk_virt(desc, &walk);
+	err = skcipher_walk_virt(&walk, req, false);
 
-	while ((nbytes = walk.nbytes)) {
+	while ((nbytes = walk.nbytes) != 0) {
 		padlock_xcrypt_cbc(walk.src.virt.addr, walk.dst.virt.addr,
 				   ctx->D, walk.iv, &ctx->cword.decrypt,
 				   nbytes / AES_BLOCK_SIZE);
 		nbytes &= AES_BLOCK_SIZE - 1;
-		err = blkcipher_walk_done(desc, &walk, nbytes);
+		err = skcipher_walk_done(&walk, nbytes);
 	}
 
 	padlock_store_cword(&ctx->cword.encrypt);
@@ -465,26 +462,20 @@ static int cbc_aes_decrypt(struct blkcipher_desc *desc,
 	return err;
 }
 
-static struct crypto_alg cbc_aes_alg = {
-	.cra_name		=	"cbc(aes)",
-	.cra_driver_name	=	"cbc-aes-padlock",
-	.cra_priority		=	PADLOCK_COMPOSITE_PRIORITY,
-	.cra_flags		=	CRYPTO_ALG_TYPE_BLKCIPHER,
-	.cra_blocksize		=	AES_BLOCK_SIZE,
-	.cra_ctxsize		=	sizeof(struct aes_ctx),
-	.cra_alignmask		=	PADLOCK_ALIGNMENT - 1,
-	.cra_type		=	&crypto_blkcipher_type,
-	.cra_module		=	THIS_MODULE,
-	.cra_u			=	{
-		.blkcipher = {
-			.min_keysize		=	AES_MIN_KEY_SIZE,
-			.max_keysize		=	AES_MAX_KEY_SIZE,
-			.ivsize			=	AES_BLOCK_SIZE,
-			.setkey	   		= 	aes_set_key,
-			.encrypt		=	cbc_aes_encrypt,
-			.decrypt		=	cbc_aes_decrypt,
-		}
-	}
+static struct skcipher_alg cbc_aes_alg = {
+	.base.cra_name		=	"cbc(aes)",
+	.base.cra_driver_name	=	"cbc-aes-padlock",
+	.base.cra_priority	=	PADLOCK_COMPOSITE_PRIORITY,
+	.base.cra_blocksize	=	AES_BLOCK_SIZE,
+	.base.cra_ctxsize	=	sizeof(struct aes_ctx),
+	.base.cra_alignmask	=	PADLOCK_ALIGNMENT - 1,
+	.base.cra_module	=	THIS_MODULE,
+	.min_keysize		=	AES_MIN_KEY_SIZE,
+	.max_keysize		=	AES_MAX_KEY_SIZE,
+	.ivsize			=	AES_BLOCK_SIZE,
+	.setkey			=	aes_set_key_skcipher,
+	.encrypt		=	cbc_aes_encrypt,
+	.decrypt		=	cbc_aes_decrypt,
 };
 
 static const struct x86_cpu_id padlock_cpu_id[] = {
@@ -506,13 +497,13 @@ static int __init padlock_init(void)
 		return -ENODEV;
 	}
 
-	if ((ret = crypto_register_alg(&aes_alg)))
+	if ((ret = crypto_register_alg(&aes_alg)) != 0)
 		goto aes_err;
 
-	if ((ret = crypto_register_alg(&ecb_aes_alg)))
+	if ((ret = crypto_register_skcipher(&ecb_aes_alg)) != 0)
 		goto ecb_aes_err;
 
-	if ((ret = crypto_register_alg(&cbc_aes_alg)))
+	if ((ret = crypto_register_skcipher(&cbc_aes_alg)) != 0)
 		goto cbc_aes_err;
 
 	printk(KERN_NOTICE PFX "Using VIA PadLock ACE for AES algorithm.\n");
@@ -527,7 +518,7 @@ static int __init padlock_init(void)
 	return ret;
 
 cbc_aes_err:
-	crypto_unregister_alg(&ecb_aes_alg);
+	crypto_unregister_skcipher(&ecb_aes_alg);
 ecb_aes_err:
 	crypto_unregister_alg(&aes_alg);
 aes_err:
@@ -537,8 +528,8 @@ static int __init padlock_init(void)
 
 static void __exit padlock_fini(void)
 {
-	crypto_unregister_alg(&cbc_aes_alg);
-	crypto_unregister_alg(&ecb_aes_alg);
+	crypto_unregister_skcipher(&cbc_aes_alg);
+	crypto_unregister_skcipher(&ecb_aes_alg);
 	crypto_unregister_alg(&aes_alg);
 }
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] crypto: padlock-aes - convert to skcipher API
  2019-10-13  4:17 [PATCH] crypto: padlock-aes - convert to skcipher API Eric Biggers
@ 2019-10-13 23:20 ` Jamie Heilman
  2019-10-14  3:12   ` Eric Biggers
  2019-10-14 12:33 ` Ard Biesheuvel
  2019-10-18  8:05 ` Herbert Xu
  2 siblings, 1 reply; 7+ messages in thread
From: Jamie Heilman @ 2019-10-13 23:20 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-crypto, Herbert Xu

Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Convert the VIA PadLock implementations of AES-ECB and AES-CBC from the
> deprecated "blkcipher" API to the "skcipher" API.  This is needed in
> order for the blkcipher API to be removed.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> 
> This is compile-tested only, as I don't have this hardware.
> If anyone has this hardware, please test it with
> CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y.

Well I gave it a spin on my Esther system against 5.3.6 but results
were somewhat obscured by the fact I seem to have other problems with
modern kernels (I'd been running Greg's 4.19 series on this system
which doesn't have the extra tests you wanted) on this hardware to the
tune of (from an unpatched 5.3.6):

 Loading compiled-in X.509 certificates
 ------------[ cut here ]------------
 WARNING: CPU: 0 PID: 1 at crypto/rsa-pkcs1pad.c:539 pkcs1pad_verify+0x2d/0xf4
 Modules linked in:
 CPU: 0 PID: 1 Comm: swapper Tainted: G                T 5.3.6 #2
 Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To be filled by O.E.M., BIOS 080014  06/01/2009
 EIP: pkcs1pad_verify+0x2d/0xf4
 Code: 57 56 53 89 c3 83 7b 1c 00 74 0e 68 c8 7a 46 c1 e8 48 43 ec ff 0f 0b eb 13 8b 53 24 85 d2 75 17 68 c8 7a 46 c1 e8 33 43 ec ff <0f> 0b 59 b8 ea ff ff ff e9 b2 00 00 00 8b 73 10 b8 ea ff ff ff 8b
 EAX: 00000024 EBX: f124a3c0 ECX: 00000100 EDX: c14fab54
 ESI: 00000000 EDI: f124a3c0 EBP: f106bd58 ESP: f106bd48
 DS: 007b ES: 007b FS: 0000 GS: 00e0 SS: 0068 EFLAGS: 00010246
 CR0: 80050033 CR2: 00000000 CR3: 0158c000 CR4: 000006b0
 Call Trace:
  public_key_verify_signature+0x1ff/0x26b
  x509_check_for_self_signed+0x9f/0xb8
  x509_cert_parse+0x149/0x179
  x509_key_preparse+0x1a/0x16d
  ? __down_read+0x26/0x29
  asymmetric_key_preparse+0x35/0x56
  key_create_or_update+0x121/0x330
  load_system_certificate_list+0x77/0xc5
  ? system_trusted_keyring_init+0x4f/0x4f
  do_one_initcall+0x7b/0x158
  kernel_init_freeable+0xd7/0x156
  ? rest_init+0x6d/0x6d
  kernel_init+0x8/0xd0
  ret_from_fork+0x33/0x40
 ---[ end trace 1ec5d41c10bd49a3 ]---
 Problem loading in-kernel X.509 certificate (-22)

That said, I get this issue with or without your patch, so I assume
it's unrelated, and probably something with c7381b01287240ab that
introduced that WARN_ON.  Anyways, I'll have to run a real bisection
on that when I have the time.

I built a patched 5.3.6 with none of the crypto bits modularized and
you can find that dmesg and config at:

http://audible.transient.net/~jamie/k/skcipher.config-5.3.6
http://audible.transient.net/~jamie/k/skcipher.dmesg

Hope that helps.

-- 
Jamie Heilman                     http://audible.transient.net/~jamie/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] crypto: padlock-aes - convert to skcipher API
  2019-10-13 23:20 ` Jamie Heilman
@ 2019-10-14  3:12   ` Eric Biggers
  2019-10-14  4:47     ` Jamie Heilman
  0 siblings, 1 reply; 7+ messages in thread
From: Eric Biggers @ 2019-10-14  3:12 UTC (permalink / raw)
  To: Jamie Heilman; +Cc: linux-crypto, Herbert Xu

On Sun, Oct 13, 2019 at 11:20:51PM +0000, Jamie Heilman wrote:
> Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Convert the VIA PadLock implementations of AES-ECB and AES-CBC from the
> > deprecated "blkcipher" API to the "skcipher" API.  This is needed in
> > order for the blkcipher API to be removed.
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> > 
> > This is compile-tested only, as I don't have this hardware.
> > If anyone has this hardware, please test it with
> > CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y.
> 
> Well I gave it a spin on my Esther system against 5.3.6 but results
> were somewhat obscured by the fact I seem to have other problems with
> modern kernels (I'd been running Greg's 4.19 series on this system
> which doesn't have the extra tests you wanted) on this hardware to the
> tune of (from an unpatched 5.3.6):
> 
>  Loading compiled-in X.509 certificates
>  ------------[ cut here ]------------
>  WARNING: CPU: 0 PID: 1 at crypto/rsa-pkcs1pad.c:539 pkcs1pad_verify+0x2d/0xf4
>  Modules linked in:
>  CPU: 0 PID: 1 Comm: swapper Tainted: G                T 5.3.6 #2
>  Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To be filled by O.E.M., BIOS 080014  06/01/2009
>  EIP: pkcs1pad_verify+0x2d/0xf4
>  Code: 57 56 53 89 c3 83 7b 1c 00 74 0e 68 c8 7a 46 c1 e8 48 43 ec ff 0f 0b eb 13 8b 53 24 85 d2 75 17 68 c8 7a 46 c1 e8 33 43 ec ff <0f> 0b 59 b8 ea ff ff ff e9 b2 00 00 00 8b 73 10 b8 ea ff ff ff 8b
>  EAX: 00000024 EBX: f124a3c0 ECX: 00000100 EDX: c14fab54
>  ESI: 00000000 EDI: f124a3c0 EBP: f106bd58 ESP: f106bd48
>  DS: 007b ES: 007b FS: 0000 GS: 00e0 SS: 0068 EFLAGS: 00010246
>  CR0: 80050033 CR2: 00000000 CR3: 0158c000 CR4: 000006b0
>  Call Trace:
>   public_key_verify_signature+0x1ff/0x26b
>   x509_check_for_self_signed+0x9f/0xb8
>   x509_cert_parse+0x149/0x179
>   x509_key_preparse+0x1a/0x16d
>   ? __down_read+0x26/0x29
>   asymmetric_key_preparse+0x35/0x56
>   key_create_or_update+0x121/0x330
>   load_system_certificate_list+0x77/0xc5
>   ? system_trusted_keyring_init+0x4f/0x4f
>   do_one_initcall+0x7b/0x158
>   kernel_init_freeable+0xd7/0x156
>   ? rest_init+0x6d/0x6d
>   kernel_init+0x8/0xd0
>   ret_from_fork+0x33/0x40
>  ---[ end trace 1ec5d41c10bd49a3 ]---
>  Problem loading in-kernel X.509 certificate (-22)
> 
> That said, I get this issue with or without your patch, so I assume
> it's unrelated, and probably something with c7381b01287240ab that
> introduced that WARN_ON.  Anyways, I'll have to run a real bisection
> on that when I have the time.

Yes, that's something unrelated.  It looks to be a bug caused by recent changes
to the asymmetric keys subsystem, triggered by not having some hash algorithm
built into the kernel (probably SHA-256).  I recommend starting a new thread for
that with:

	David Howells <dhowells@redhat.com>
	Vitaly Chikunov <vt@altlinux.org>
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
	keyrings@vger.kernel.org
	linux-crypto@vger.kernel.org

> 
> I built a patched 5.3.6 with none of the crypto bits modularized and
> you can find that dmesg and config at:
> 
> http://audible.transient.net/~jamie/k/skcipher.config-5.3.6
> http://audible.transient.net/~jamie/k/skcipher.dmesg
> 

Great, I don't see any test failures in the log.  Just to double check, you had
applied both Ard's patch and this one, right?:

	crypto: geode-aes - switch to skcipher for cbc(aes) fallback
	crypto: geode-aes - convert to skcipher API and make thread-safe

- Eric

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] crypto: padlock-aes - convert to skcipher API
  2019-10-14  3:12   ` Eric Biggers
@ 2019-10-14  4:47     ` Jamie Heilman
  2019-10-14  4:54       ` Eric Biggers
  0 siblings, 1 reply; 7+ messages in thread
From: Jamie Heilman @ 2019-10-14  4:47 UTC (permalink / raw)
  To: linux-crypto, Herbert Xu

Eric Biggers wrote:
> > I built a patched 5.3.6 with none of the crypto bits modularized
> > and you can find that dmesg and config at:
> > 
> > http://audible.transient.net/~jamie/k/skcipher.config-5.3.6
> > http://audible.transient.net/~jamie/k/skcipher.dmesg
> > 
> 
> Great, I don't see any test failures in the log.  Just to double
> check, you had applied both Ard's patch and this one, right?:
> 
> 	crypto: geode-aes - switch to skcipher for cbc(aes) fallback
> 	crypto: geode-aes - convert to skcipher API and make thread-safe

Er, no?  Just the VIA padlock-aes.c change from you on top of a stock
5.3.6 tree, nothing from Ard.  No geode here.

-- 
Jamie Heilman                     http://audible.transient.net/~jamie/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] crypto: padlock-aes - convert to skcipher API
  2019-10-14  4:47     ` Jamie Heilman
@ 2019-10-14  4:54       ` Eric Biggers
  0 siblings, 0 replies; 7+ messages in thread
From: Eric Biggers @ 2019-10-14  4:54 UTC (permalink / raw)
  To: Jamie Heilman; +Cc: linux-crypto, Herbert Xu

On Mon, Oct 14, 2019 at 04:47:27AM +0000, Jamie Heilman wrote:
> Eric Biggers wrote:
> > > I built a patched 5.3.6 with none of the crypto bits modularized
> > > and you can find that dmesg and config at:
> > > 
> > > http://audible.transient.net/~jamie/k/skcipher.config-5.3.6
> > > http://audible.transient.net/~jamie/k/skcipher.dmesg
> > > 
> > 
> > Great, I don't see any test failures in the log.  Just to double
> > check, you had applied both Ard's patch and this one, right?:
> > 
> > 	crypto: geode-aes - switch to skcipher for cbc(aes) fallback
> > 	crypto: geode-aes - convert to skcipher API and make thread-safe
> 
> Er, no?  Just the VIA padlock-aes.c change from you on top of a stock
> 5.3.6 tree, nothing from Ard.  No geode here.
> 

Oops, sorry, I confused this with a different driver.  Yes, if you just applied
this padlock-aes patch it's good.

Thanks for testing!

- Eric

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] crypto: padlock-aes - convert to skcipher API
  2019-10-13  4:17 [PATCH] crypto: padlock-aes - convert to skcipher API Eric Biggers
  2019-10-13 23:20 ` Jamie Heilman
@ 2019-10-14 12:33 ` Ard Biesheuvel
  2019-10-18  8:05 ` Herbert Xu
  2 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2019-10-14 12:33 UTC (permalink / raw)
  To: Eric Biggers
  Cc: open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu,
	Jamie Heilman

On Sun, 13 Oct 2019 at 06:19, Eric Biggers <ebiggers@kernel.org> wrote:
>
> From: Eric Biggers <ebiggers@google.com>
>
> Convert the VIA PadLock implementations of AES-ECB and AES-CBC from the
> deprecated "blkcipher" API to the "skcipher" API.  This is needed in
> order for the blkcipher API to be removed.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>
> This is compile-tested only, as I don't have this hardware.
> If anyone has this hardware, please test it with
> CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y.
>
>  drivers/crypto/padlock-aes.c | 157 +++++++++++++++++------------------
>  1 file changed, 74 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/crypto/padlock-aes.c b/drivers/crypto/padlock-aes.c
> index 8a0661250078..c5b60f50e1b5 100644
> --- a/drivers/crypto/padlock-aes.c
> +++ b/drivers/crypto/padlock-aes.c
> @@ -10,6 +10,7 @@
>
>  #include <crypto/algapi.h>
>  #include <crypto/aes.h>
> +#include <crypto/internal/skcipher.h>
>  #include <crypto/padlock.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
> @@ -97,9 +98,9 @@ static inline struct aes_ctx *aes_ctx(struct crypto_tfm *tfm)
>         return aes_ctx_common(crypto_tfm_ctx(tfm));
>  }
>
> -static inline struct aes_ctx *blk_aes_ctx(struct crypto_blkcipher *tfm)
> +static inline struct aes_ctx *skcipher_aes_ctx(struct crypto_skcipher *tfm)
>  {
> -       return aes_ctx_common(crypto_blkcipher_ctx(tfm));
> +       return aes_ctx_common(crypto_skcipher_ctx(tfm));
>  }
>
>  static int aes_set_key(struct crypto_tfm *tfm, const u8 *in_key,
> @@ -162,6 +163,12 @@ static int aes_set_key(struct crypto_tfm *tfm, const u8 *in_key,
>         return 0;
>  }
>
> +static int aes_set_key_skcipher(struct crypto_skcipher *tfm, const u8 *in_key,
> +                               unsigned int key_len)
> +{
> +       return aes_set_key(crypto_skcipher_tfm(tfm), in_key, key_len);
> +}
> +
>  /* ====== Encryption/decryption routines ====== */
>
>  /* These are the real call to PadLock. */
> @@ -338,25 +345,24 @@ static struct crypto_alg aes_alg = {
>         }
>  };
>
> -static int ecb_aes_encrypt(struct blkcipher_desc *desc,
> -                          struct scatterlist *dst, struct scatterlist *src,
> -                          unsigned int nbytes)
> +static int ecb_aes_encrypt(struct skcipher_request *req)
>  {
> -       struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
> -       struct blkcipher_walk walk;
> +       struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
> +       struct aes_ctx *ctx = skcipher_aes_ctx(tfm);
> +       struct skcipher_walk walk;
> +       unsigned int nbytes;
>         int err;
>
>         padlock_reset_key(&ctx->cword.encrypt);
>
> -       blkcipher_walk_init(&walk, dst, src, nbytes);
> -       err = blkcipher_walk_virt(desc, &walk);
> +       err = skcipher_walk_virt(&walk, req, false);
>
> -       while ((nbytes = walk.nbytes)) {
> +       while ((nbytes = walk.nbytes) != 0) {
>                 padlock_xcrypt_ecb(walk.src.virt.addr, walk.dst.virt.addr,
>                                    ctx->E, &ctx->cword.encrypt,
>                                    nbytes / AES_BLOCK_SIZE);
>                 nbytes &= AES_BLOCK_SIZE - 1;
> -               err = blkcipher_walk_done(desc, &walk, nbytes);
> +               err = skcipher_walk_done(&walk, nbytes);
>         }
>
>         padlock_store_cword(&ctx->cword.encrypt);
> @@ -364,25 +370,24 @@ static int ecb_aes_encrypt(struct blkcipher_desc *desc,
>         return err;
>  }
>
> -static int ecb_aes_decrypt(struct blkcipher_desc *desc,
> -                          struct scatterlist *dst, struct scatterlist *src,
> -                          unsigned int nbytes)
> +static int ecb_aes_decrypt(struct skcipher_request *req)
>  {
> -       struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
> -       struct blkcipher_walk walk;
> +       struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
> +       struct aes_ctx *ctx = skcipher_aes_ctx(tfm);
> +       struct skcipher_walk walk;
> +       unsigned int nbytes;
>         int err;
>
>         padlock_reset_key(&ctx->cword.decrypt);
>
> -       blkcipher_walk_init(&walk, dst, src, nbytes);
> -       err = blkcipher_walk_virt(desc, &walk);
> +       err = skcipher_walk_virt(&walk, req, false);
>
> -       while ((nbytes = walk.nbytes)) {
> +       while ((nbytes = walk.nbytes) != 0) {
>                 padlock_xcrypt_ecb(walk.src.virt.addr, walk.dst.virt.addr,
>                                    ctx->D, &ctx->cword.decrypt,
>                                    nbytes / AES_BLOCK_SIZE);
>                 nbytes &= AES_BLOCK_SIZE - 1;
> -               err = blkcipher_walk_done(desc, &walk, nbytes);
> +               err = skcipher_walk_done(&walk, nbytes);
>         }
>
>         padlock_store_cword(&ctx->cword.encrypt);
> @@ -390,48 +395,41 @@ static int ecb_aes_decrypt(struct blkcipher_desc *desc,
>         return err;
>  }
>
> -static struct crypto_alg ecb_aes_alg = {
> -       .cra_name               =       "ecb(aes)",
> -       .cra_driver_name        =       "ecb-aes-padlock",
> -       .cra_priority           =       PADLOCK_COMPOSITE_PRIORITY,
> -       .cra_flags              =       CRYPTO_ALG_TYPE_BLKCIPHER,
> -       .cra_blocksize          =       AES_BLOCK_SIZE,
> -       .cra_ctxsize            =       sizeof(struct aes_ctx),
> -       .cra_alignmask          =       PADLOCK_ALIGNMENT - 1,
> -       .cra_type               =       &crypto_blkcipher_type,
> -       .cra_module             =       THIS_MODULE,
> -       .cra_u                  =       {
> -               .blkcipher = {
> -                       .min_keysize            =       AES_MIN_KEY_SIZE,
> -                       .max_keysize            =       AES_MAX_KEY_SIZE,
> -                       .setkey                 =       aes_set_key,
> -                       .encrypt                =       ecb_aes_encrypt,
> -                       .decrypt                =       ecb_aes_decrypt,
> -               }
> -       }
> +static struct skcipher_alg ecb_aes_alg = {
> +       .base.cra_name          =       "ecb(aes)",
> +       .base.cra_driver_name   =       "ecb-aes-padlock",
> +       .base.cra_priority      =       PADLOCK_COMPOSITE_PRIORITY,
> +       .base.cra_blocksize     =       AES_BLOCK_SIZE,
> +       .base.cra_ctxsize       =       sizeof(struct aes_ctx),
> +       .base.cra_alignmask     =       PADLOCK_ALIGNMENT - 1,
> +       .base.cra_module        =       THIS_MODULE,
> +       .min_keysize            =       AES_MIN_KEY_SIZE,
> +       .max_keysize            =       AES_MAX_KEY_SIZE,
> +       .setkey                 =       aes_set_key_skcipher,
> +       .encrypt                =       ecb_aes_encrypt,
> +       .decrypt                =       ecb_aes_decrypt,
>  };
>
> -static int cbc_aes_encrypt(struct blkcipher_desc *desc,
> -                          struct scatterlist *dst, struct scatterlist *src,
> -                          unsigned int nbytes)
> +static int cbc_aes_encrypt(struct skcipher_request *req)
>  {
> -       struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
> -       struct blkcipher_walk walk;
> +       struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
> +       struct aes_ctx *ctx = skcipher_aes_ctx(tfm);
> +       struct skcipher_walk walk;
> +       unsigned int nbytes;
>         int err;
>
>         padlock_reset_key(&ctx->cword.encrypt);
>
> -       blkcipher_walk_init(&walk, dst, src, nbytes);
> -       err = blkcipher_walk_virt(desc, &walk);
> +       err = skcipher_walk_virt(&walk, req, false);
>
> -       while ((nbytes = walk.nbytes)) {
> +       while ((nbytes = walk.nbytes) != 0) {
>                 u8 *iv = padlock_xcrypt_cbc(walk.src.virt.addr,
>                                             walk.dst.virt.addr, ctx->E,
>                                             walk.iv, &ctx->cword.encrypt,
>                                             nbytes / AES_BLOCK_SIZE);
>                 memcpy(walk.iv, iv, AES_BLOCK_SIZE);
>                 nbytes &= AES_BLOCK_SIZE - 1;
> -               err = blkcipher_walk_done(desc, &walk, nbytes);
> +               err = skcipher_walk_done(&walk, nbytes);
>         }
>
>         padlock_store_cword(&ctx->cword.decrypt);
> @@ -439,25 +437,24 @@ static int cbc_aes_encrypt(struct blkcipher_desc *desc,
>         return err;
>  }
>
> -static int cbc_aes_decrypt(struct blkcipher_desc *desc,
> -                          struct scatterlist *dst, struct scatterlist *src,
> -                          unsigned int nbytes)
> +static int cbc_aes_decrypt(struct skcipher_request *req)
>  {
> -       struct aes_ctx *ctx = blk_aes_ctx(desc->tfm);
> -       struct blkcipher_walk walk;
> +       struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
> +       struct aes_ctx *ctx = skcipher_aes_ctx(tfm);
> +       struct skcipher_walk walk;
> +       unsigned int nbytes;
>         int err;
>
>         padlock_reset_key(&ctx->cword.encrypt);
>
> -       blkcipher_walk_init(&walk, dst, src, nbytes);
> -       err = blkcipher_walk_virt(desc, &walk);
> +       err = skcipher_walk_virt(&walk, req, false);
>
> -       while ((nbytes = walk.nbytes)) {
> +       while ((nbytes = walk.nbytes) != 0) {
>                 padlock_xcrypt_cbc(walk.src.virt.addr, walk.dst.virt.addr,
>                                    ctx->D, walk.iv, &ctx->cword.decrypt,
>                                    nbytes / AES_BLOCK_SIZE);
>                 nbytes &= AES_BLOCK_SIZE - 1;
> -               err = blkcipher_walk_done(desc, &walk, nbytes);
> +               err = skcipher_walk_done(&walk, nbytes);
>         }
>
>         padlock_store_cword(&ctx->cword.encrypt);
> @@ -465,26 +462,20 @@ static int cbc_aes_decrypt(struct blkcipher_desc *desc,
>         return err;
>  }
>
> -static struct crypto_alg cbc_aes_alg = {
> -       .cra_name               =       "cbc(aes)",
> -       .cra_driver_name        =       "cbc-aes-padlock",
> -       .cra_priority           =       PADLOCK_COMPOSITE_PRIORITY,
> -       .cra_flags              =       CRYPTO_ALG_TYPE_BLKCIPHER,
> -       .cra_blocksize          =       AES_BLOCK_SIZE,
> -       .cra_ctxsize            =       sizeof(struct aes_ctx),
> -       .cra_alignmask          =       PADLOCK_ALIGNMENT - 1,
> -       .cra_type               =       &crypto_blkcipher_type,
> -       .cra_module             =       THIS_MODULE,
> -       .cra_u                  =       {
> -               .blkcipher = {
> -                       .min_keysize            =       AES_MIN_KEY_SIZE,
> -                       .max_keysize            =       AES_MAX_KEY_SIZE,
> -                       .ivsize                 =       AES_BLOCK_SIZE,
> -                       .setkey                 =       aes_set_key,
> -                       .encrypt                =       cbc_aes_encrypt,
> -                       .decrypt                =       cbc_aes_decrypt,
> -               }
> -       }
> +static struct skcipher_alg cbc_aes_alg = {
> +       .base.cra_name          =       "cbc(aes)",
> +       .base.cra_driver_name   =       "cbc-aes-padlock",
> +       .base.cra_priority      =       PADLOCK_COMPOSITE_PRIORITY,
> +       .base.cra_blocksize     =       AES_BLOCK_SIZE,
> +       .base.cra_ctxsize       =       sizeof(struct aes_ctx),
> +       .base.cra_alignmask     =       PADLOCK_ALIGNMENT - 1,
> +       .base.cra_module        =       THIS_MODULE,
> +       .min_keysize            =       AES_MIN_KEY_SIZE,
> +       .max_keysize            =       AES_MAX_KEY_SIZE,
> +       .ivsize                 =       AES_BLOCK_SIZE,
> +       .setkey                 =       aes_set_key_skcipher,
> +       .encrypt                =       cbc_aes_encrypt,
> +       .decrypt                =       cbc_aes_decrypt,
>  };
>
>  static const struct x86_cpu_id padlock_cpu_id[] = {
> @@ -506,13 +497,13 @@ static int __init padlock_init(void)
>                 return -ENODEV;
>         }
>
> -       if ((ret = crypto_register_alg(&aes_alg)))
> +       if ((ret = crypto_register_alg(&aes_alg)) != 0)
>                 goto aes_err;
>
> -       if ((ret = crypto_register_alg(&ecb_aes_alg)))
> +       if ((ret = crypto_register_skcipher(&ecb_aes_alg)) != 0)
>                 goto ecb_aes_err;
>
> -       if ((ret = crypto_register_alg(&cbc_aes_alg)))
> +       if ((ret = crypto_register_skcipher(&cbc_aes_alg)) != 0)
>                 goto cbc_aes_err;
>
>         printk(KERN_NOTICE PFX "Using VIA PadLock ACE for AES algorithm.\n");
> @@ -527,7 +518,7 @@ static int __init padlock_init(void)
>         return ret;
>
>  cbc_aes_err:
> -       crypto_unregister_alg(&ecb_aes_alg);
> +       crypto_unregister_skcipher(&ecb_aes_alg);
>  ecb_aes_err:
>         crypto_unregister_alg(&aes_alg);
>  aes_err:
> @@ -537,8 +528,8 @@ static int __init padlock_init(void)
>
>  static void __exit padlock_fini(void)
>  {
> -       crypto_unregister_alg(&cbc_aes_alg);
> -       crypto_unregister_alg(&ecb_aes_alg);
> +       crypto_unregister_skcipher(&cbc_aes_alg);
> +       crypto_unregister_skcipher(&ecb_aes_alg);
>         crypto_unregister_alg(&aes_alg);
>  }
>
> --
> 2.23.0
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] crypto: padlock-aes - convert to skcipher API
  2019-10-13  4:17 [PATCH] crypto: padlock-aes - convert to skcipher API Eric Biggers
  2019-10-13 23:20 ` Jamie Heilman
  2019-10-14 12:33 ` Ard Biesheuvel
@ 2019-10-18  8:05 ` Herbert Xu
  2 siblings, 0 replies; 7+ messages in thread
From: Herbert Xu @ 2019-10-18  8:05 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-crypto, Jamie Heilman

On Sat, Oct 12, 2019 at 09:17:41PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Convert the VIA PadLock implementations of AES-ECB and AES-CBC from the
> deprecated "blkcipher" API to the "skcipher" API.  This is needed in
> order for the blkcipher API to be removed.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> 
> This is compile-tested only, as I don't have this hardware.
> If anyone has this hardware, please test it with
> CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y.
> 
>  drivers/crypto/padlock-aes.c | 157 +++++++++++++++++------------------
>  1 file changed, 74 insertions(+), 83 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-10-18  8:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-13  4:17 [PATCH] crypto: padlock-aes - convert to skcipher API Eric Biggers
2019-10-13 23:20 ` Jamie Heilman
2019-10-14  3:12   ` Eric Biggers
2019-10-14  4:47     ` Jamie Heilman
2019-10-14  4:54       ` Eric Biggers
2019-10-14 12:33 ` Ard Biesheuvel
2019-10-18  8:05 ` Herbert Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.