[PATCH v2 0/6] crypto: remove old way of allocating and freeing instances

[PATCH v2 0/6] crypto: remove old way of allocating and freeing instances

[Eric Biggers]

This series makes all crypto templates use the new way of freeing
instances where a ->free() method is installed to the instance struct
itself.  This replaces the weakly-typed method crypto_template::free().

skcipher and akcipher were already using the new way, while aead was
mostly but not always using the new way.  shash and ahash were using the
old way.  This series eliminates this inconsistency (and the redundant
code associated with it) by making everyone use the new way.

The last patch adds registration-time checks which verify that all
instances really have a ->free() method.

This series is an internal cleanup only; there are no changes for users
of the crypto API.

This series is based on top of my other series
"[PATCH v2 00/28] crypto: template instantiation cleanup".

Changed v1 => v2:

  - Rebased onto v2 of the other series.

Eric Biggers (6):
  crypto: hash - add support for new way of freeing instances
  crypto: geniv - convert to new way of freeing instances
  crypto: cryptd - convert to new way of freeing instances
  crypto: shash - convert shash_free_instance() to new style
  crypto: algapi - remove crypto_template::{alloc,free}()
  crypto: algapi - enforce that all instances have a ->free() method

 crypto/aead.c                   |  8 +++----
 crypto/ahash.c                  | 11 +++++++++
 crypto/akcipher.c               |  2 ++
 crypto/algapi.c                 |  5 ----
 crypto/algboss.c                | 12 +---------
 crypto/ccm.c                    |  5 ++--
 crypto/cmac.c                   |  5 ++--
 crypto/cryptd.c                 | 42 ++++++++++++++++-----------------
 crypto/echainiv.c               | 20 ++++------------
 crypto/geniv.c                  | 15 ++++++------
 crypto/hmac.c                   |  5 ++--
 crypto/seqiv.c                  | 20 ++++------------
 crypto/shash.c                  | 19 +++++++++++----
 crypto/skcipher.c               |  3 +++
 crypto/vmac.c                   |  5 ++--
 crypto/xcbc.c                   |  5 ++--
 include/crypto/algapi.h         |  2 --
 include/crypto/internal/geniv.h |  1 -
 include/crypto/internal/hash.h  |  4 +++-
 19 files changed, 89 insertions(+), 100 deletions(-)

-- 
2.24.1

[PATCH v2 1/6] crypto: hash - add support for new way of freeing instances

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Add support to shash and ahash for the new way of freeing instances
(already used for skcipher, aead, and akcipher) where a ->free() method
is installed to the instance struct itself.  These methods are more
strongly-typed than crypto_template::free(), which they replace.

This will allow removing support for the old way of freeing instances.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/ahash.c                 | 13 +++++++++++++
 crypto/shash.c                 | 13 +++++++++++++
 include/crypto/internal/hash.h |  2 ++
 3 files changed, 28 insertions(+)

diff --git a/crypto/ahash.c b/crypto/ahash.c
index c77717fcea8e..61e374d76b04 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -511,6 +511,18 @@ static unsigned int crypto_ahash_extsize(struct crypto_alg *alg)
 	return crypto_alg_extsize(alg);
 }
 
+static void crypto_ahash_free_instance(struct crypto_instance *inst)
+{
+	struct ahash_instance *ahash = ahash_instance(inst);
+
+	if (!ahash->free) {
+		inst->tmpl->free(inst);
+		return;
+	}
+
+	ahash->free(ahash);
+}
+
 #ifdef CONFIG_NET
 static int crypto_ahash_report(struct sk_buff *skb, struct crypto_alg *alg)
 {
@@ -547,6 +559,7 @@ static void crypto_ahash_show(struct seq_file *m, struct crypto_alg *alg)
 static const struct crypto_type crypto_ahash_type = {
 	.extsize = crypto_ahash_extsize,
 	.init_tfm = crypto_ahash_init_tfm,
+	.free = crypto_ahash_free_instance,
 #ifdef CONFIG_PROC_FS
 	.show = crypto_ahash_show,
 #endif
diff --git a/crypto/shash.c b/crypto/shash.c
index 4d6ccb59e126..2f6adb49727b 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -423,6 +423,18 @@ static int crypto_shash_init_tfm(struct crypto_tfm *tfm)
 	return 0;
 }
 
+static void crypto_shash_free_instance(struct crypto_instance *inst)
+{
+	struct shash_instance *shash = shash_instance(inst);
+
+	if (!shash->free) {
+		inst->tmpl->free(inst);
+		return;
+	}
+
+	shash->free(shash);
+}
+
 #ifdef CONFIG_NET
 static int crypto_shash_report(struct sk_buff *skb, struct crypto_alg *alg)
 {
@@ -459,6 +471,7 @@ static void crypto_shash_show(struct seq_file *m, struct crypto_alg *alg)
 static const struct crypto_type crypto_shash_type = {
 	.extsize = crypto_alg_extsize,
 	.init_tfm = crypto_shash_init_tfm,
+	.free = crypto_shash_free_instance,
 #ifdef CONFIG_PROC_FS
 	.show = crypto_shash_show,
 #endif
diff --git a/include/crypto/internal/hash.h b/include/crypto/internal/hash.h
index c84b7cb29887..c550386221bb 100644
--- a/include/crypto/internal/hash.h
+++ b/include/crypto/internal/hash.h
@@ -30,6 +30,7 @@ struct crypto_hash_walk {
 };
 
 struct ahash_instance {
+	void (*free)(struct ahash_instance *inst);
 	union {
 		struct {
 			char head[offsetof(struct ahash_alg, halg.base)];
@@ -40,6 +41,7 @@ struct ahash_instance {
 };
 
 struct shash_instance {
+	void (*free)(struct shash_instance *inst);
 	union {
 		struct {
 			char head[offsetof(struct shash_alg, base)];
-- 
2.24.1

[PATCH v2 2/6] crypto: geniv - convert to new way of freeing instances

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Convert the "seqiv" template to the new way of freeing instances where a
->free() method is installed to the instance struct itself.  Also remove
the unused implementation of the old way of freeing instances from the
"echainiv" template, since it's already using the new way too.

In doing this, also simplify the code by making the helper function
aead_geniv_alloc() install the ->free() method, instead of making seqiv
and echainiv do this themselves.  This is analogous to how
skcipher_alloc_instance_simple() works.

This will allow removing support for the old way of freeing instances.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/echainiv.c               | 20 ++++----------------
 crypto/geniv.c                  | 15 ++++++++-------
 crypto/seqiv.c                  | 20 ++++----------------
 include/crypto/internal/geniv.h |  1 -
 4 files changed, 16 insertions(+), 40 deletions(-)

diff --git a/crypto/echainiv.c b/crypto/echainiv.c
index a49cbf7b0929..4a2f02baba14 100644
--- a/crypto/echainiv.c
+++ b/crypto/echainiv.c
@@ -133,29 +133,17 @@ static int echainiv_aead_create(struct crypto_template *tmpl,
 	inst->alg.base.cra_ctxsize = sizeof(struct aead_geniv_ctx);
 	inst->alg.base.cra_ctxsize += inst->alg.ivsize;
 
-	inst->free = aead_geniv_free;
-
 	err = aead_register_instance(tmpl, inst);
-	if (err)
-		goto free_inst;
-
-out:
-	return err;
-
+	if (err) {
 free_inst:
-	aead_geniv_free(inst);
-	goto out;
-}
-
-static void echainiv_free(struct crypto_instance *inst)
-{
-	aead_geniv_free(aead_instance(inst));
+		inst->free(inst);
+	}
+	return err;
 }
 
 static struct crypto_template echainiv_tmpl = {
 	.name = "echainiv",
 	.create = echainiv_aead_create,
-	.free = echainiv_free,
 	.module = THIS_MODULE,
 };
 
diff --git a/crypto/geniv.c b/crypto/geniv.c
index 7afa48414f3a..dbcc640274cd 100644
--- a/crypto/geniv.c
+++ b/crypto/geniv.c
@@ -32,6 +32,12 @@ static int aead_geniv_setauthsize(struct crypto_aead *tfm,
 	return crypto_aead_setauthsize(ctx->child, authsize);
 }
 
+static void aead_geniv_free(struct aead_instance *inst)
+{
+	crypto_drop_aead(aead_instance_ctx(inst));
+	kfree(inst);
+}
+
 struct aead_instance *aead_geniv_alloc(struct crypto_template *tmpl,
 				       struct rtattr **tb, u32 type, u32 mask)
 {
@@ -100,6 +106,8 @@ struct aead_instance *aead_geniv_alloc(struct crypto_template *tmpl,
 	inst->alg.ivsize = ivsize;
 	inst->alg.maxauthsize = maxauthsize;
 
+	inst->free = aead_geniv_free;
+
 out:
 	return inst;
 
@@ -112,13 +120,6 @@ struct aead_instance *aead_geniv_alloc(struct crypto_template *tmpl,
 }
 EXPORT_SYMBOL_GPL(aead_geniv_alloc);
 
-void aead_geniv_free(struct aead_instance *inst)
-{
-	crypto_drop_aead(aead_instance_ctx(inst));
-	kfree(inst);
-}
-EXPORT_SYMBOL_GPL(aead_geniv_free);
-
 int aead_init_geniv(struct crypto_aead *aead)
 {
 	struct aead_geniv_ctx *ctx = crypto_aead_ctx(aead);
diff --git a/crypto/seqiv.c b/crypto/seqiv.c
index 96d222c32acc..f124b9b54e15 100644
--- a/crypto/seqiv.c
+++ b/crypto/seqiv.c
@@ -18,8 +18,6 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 
-static void seqiv_free(struct crypto_instance *inst);
-
 static void seqiv_aead_encrypt_complete2(struct aead_request *req, int err)
 {
 	struct aead_request *subreq = aead_request_ctx(req);
@@ -159,15 +157,11 @@ static int seqiv_aead_create(struct crypto_template *tmpl, struct rtattr **tb)
 	inst->alg.base.cra_ctxsize += inst->alg.ivsize;
 
 	err = aead_register_instance(tmpl, inst);
-	if (err)
-		goto free_inst;
-
-out:
-	return err;
-
+	if (err) {
 free_inst:
-	aead_geniv_free(inst);
-	goto out;
+		inst->free(inst);
+	}
+	return err;
 }
 
 static int seqiv_create(struct crypto_template *tmpl, struct rtattr **tb)
@@ -184,15 +178,9 @@ static int seqiv_create(struct crypto_template *tmpl, struct rtattr **tb)
 	return seqiv_aead_create(tmpl, tb);
 }
 
-static void seqiv_free(struct crypto_instance *inst)
-{
-	aead_geniv_free(aead_instance(inst));
-}
-
 static struct crypto_template seqiv_tmpl = {
 	.name = "seqiv",
 	.create = seqiv_create,
-	.free = seqiv_free,
 	.module = THIS_MODULE,
 };
 
diff --git a/include/crypto/internal/geniv.h b/include/crypto/internal/geniv.h
index 0108c0c7b2ed..229d37681a9d 100644
--- a/include/crypto/internal/geniv.h
+++ b/include/crypto/internal/geniv.h
@@ -21,7 +21,6 @@ struct aead_geniv_ctx {
 
 struct aead_instance *aead_geniv_alloc(struct crypto_template *tmpl,
 				       struct rtattr **tb, u32 type, u32 mask);
-void aead_geniv_free(struct aead_instance *inst);
 int aead_init_geniv(struct crypto_aead *tfm);
 void aead_exit_geniv(struct crypto_aead *tfm);
 
-- 
2.24.1

[PATCH v2 3/6] crypto: cryptd - convert to new way of freeing instances

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Convert the "cryptd" template to the new way of freeing instances, where
a ->free() method is installed to the instance struct itself.  This
replaces the weakly-typed method crypto_template::free().

This will allow removing support for the old way of freeing instances.

Note that the 'default' case in cryptd_free() was already unreachable.
So, we aren't missing anything by keeping only the ahash and aead parts.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/cryptd.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/crypto/cryptd.c b/crypto/cryptd.c
index 3224f142c824..f29369b77d3b 100644
--- a/crypto/cryptd.c
+++ b/crypto/cryptd.c
@@ -640,6 +640,14 @@ static int cryptd_hash_import(struct ahash_request *req, const void *in)
 	return crypto_shash_import(desc, in);
 }
 
+static void cryptd_hash_free(struct ahash_instance *inst)
+{
+	struct hashd_instance_ctx *ctx = ahash_instance_ctx(inst);
+
+	crypto_drop_shash(&ctx->spawn);
+	kfree(inst);
+}
+
 static int cryptd_create_hash(struct crypto_template *tmpl, struct rtattr **tb,
 			      struct cryptd_queue *queue)
 {
@@ -690,6 +698,8 @@ static int cryptd_create_hash(struct crypto_template *tmpl, struct rtattr **tb,
 		inst->alg.setkey = cryptd_hash_setkey;
 	inst->alg.digest = cryptd_hash_digest_enqueue;
 
+	inst->free = cryptd_hash_free;
+
 	err = ahash_register_instance(tmpl, inst);
 	if (err) {
 err_free_inst:
@@ -817,6 +827,14 @@ static void cryptd_aead_exit_tfm(struct crypto_aead *tfm)
 	crypto_free_aead(ctx->child);
 }
 
+static void cryptd_aead_free(struct aead_instance *inst)
+{
+	struct aead_instance_ctx *ctx = aead_instance_ctx(inst);
+
+	crypto_drop_aead(&ctx->aead_spawn);
+	kfree(inst);
+}
+
 static int cryptd_create_aead(struct crypto_template *tmpl,
 		              struct rtattr **tb,
 			      struct cryptd_queue *queue)
@@ -866,6 +884,8 @@ static int cryptd_create_aead(struct crypto_template *tmpl,
 	inst->alg.encrypt = cryptd_aead_encrypt_enqueue;
 	inst->alg.decrypt = cryptd_aead_decrypt_enqueue;
 
+	inst->free = cryptd_aead_free;
+
 	err = aead_register_instance(tmpl, inst);
 	if (err) {
 out_drop_aead:
@@ -898,31 +918,9 @@ static int cryptd_create(struct crypto_template *tmpl, struct rtattr **tb)
 	return -EINVAL;
 }
 
-static void cryptd_free(struct crypto_instance *inst)
-{
-	struct cryptd_instance_ctx *ctx = crypto_instance_ctx(inst);
-	struct hashd_instance_ctx *hctx = crypto_instance_ctx(inst);
-	struct aead_instance_ctx *aead_ctx = crypto_instance_ctx(inst);
-
-	switch (inst->alg.cra_flags & CRYPTO_ALG_TYPE_MASK) {
-	case CRYPTO_ALG_TYPE_AHASH:
-		crypto_drop_shash(&hctx->spawn);
-		kfree(ahash_instance(inst));
-		return;
-	case CRYPTO_ALG_TYPE_AEAD:
-		crypto_drop_aead(&aead_ctx->aead_spawn);
-		kfree(aead_instance(inst));
-		return;
-	default:
-		crypto_drop_spawn(&ctx->spawn);
-		kfree(inst);
-	}
-}
-
 static struct crypto_template cryptd_tmpl = {
 	.name = "cryptd",
 	.create = cryptd_create,
-	.free = cryptd_free,
 	.module = THIS_MODULE,
 };
 
-- 
2.24.1

[PATCH v2 4/6] crypto: shash - convert shash_free_instance() to new style

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Convert shash_free_instance() and its users to the new way of freeing
instances, where a ->free() method is installed to the instance struct
itself.  This replaces the weakly-typed method crypto_template::free().

This will allow removing support for the old way of freeing instances.

Also give shash_free_instance() a more descriptive name to reflect that
it's only for instances with a single spawn, not for any instance.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/ccm.c                   | 5 +++--
 crypto/cmac.c                  | 5 +++--
 crypto/hmac.c                  | 5 +++--
 crypto/shash.c                 | 8 ++++----
 crypto/vmac.c                  | 5 +++--
 crypto/xcbc.c                  | 5 +++--
 include/crypto/internal/hash.h | 2 +-
 7 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/crypto/ccm.c b/crypto/ccm.c
index c7a887565c51..6bb08cee816e 100644
--- a/crypto/ccm.c
+++ b/crypto/ccm.c
@@ -939,10 +939,12 @@ static int cbcmac_create(struct crypto_template *tmpl, struct rtattr **tb)
 	inst->alg.final = crypto_cbcmac_digest_final;
 	inst->alg.setkey = crypto_cbcmac_digest_setkey;
 
+	inst->free = shash_free_singlespawn_instance;
+
 	err = shash_register_instance(tmpl, inst);
 	if (err) {
 err_free_inst:
-		shash_free_instance(shash_crypto_instance(inst));
+		shash_free_singlespawn_instance(inst);
 	}
 	return err;
 }
@@ -951,7 +953,6 @@ static struct crypto_template crypto_ccm_tmpls[] = {
 	{
 		.name = "cbcmac",
 		.create = cbcmac_create,
-		.free = shash_free_instance,
 		.module = THIS_MODULE,
 	}, {
 		.name = "ccm_base",
diff --git a/crypto/cmac.c b/crypto/cmac.c
index 58dc644416bb..143a6544c873 100644
--- a/crypto/cmac.c
+++ b/crypto/cmac.c
@@ -280,10 +280,12 @@ static int cmac_create(struct crypto_template *tmpl, struct rtattr **tb)
 	inst->alg.final = crypto_cmac_digest_final;
 	inst->alg.setkey = crypto_cmac_digest_setkey;
 
+	inst->free = shash_free_singlespawn_instance;
+
 	err = shash_register_instance(tmpl, inst);
 	if (err) {
 err_free_inst:
-		shash_free_instance(shash_crypto_instance(inst));
+		shash_free_singlespawn_instance(inst);
 	}
 	return err;
 }
@@ -291,7 +293,6 @@ static int cmac_create(struct crypto_template *tmpl, struct rtattr **tb)
 static struct crypto_template crypto_cmac_tmpl = {
 	.name = "cmac",
 	.create = cmac_create,
-	.free = shash_free_instance,
 	.module = THIS_MODULE,
 };
 
diff --git a/crypto/hmac.c b/crypto/hmac.c
index 0a42b7075763..e38bfb948278 100644
--- a/crypto/hmac.c
+++ b/crypto/hmac.c
@@ -224,10 +224,12 @@ static int hmac_create(struct crypto_template *tmpl, struct rtattr **tb)
 	inst->alg.init_tfm = hmac_init_tfm;
 	inst->alg.exit_tfm = hmac_exit_tfm;
 
+	inst->free = shash_free_singlespawn_instance;
+
 	err = shash_register_instance(tmpl, inst);
 	if (err) {
 err_free_inst:
-		shash_free_instance(shash_crypto_instance(inst));
+		shash_free_singlespawn_instance(inst);
 	}
 	return err;
 }
@@ -235,7 +237,6 @@ static int hmac_create(struct crypto_template *tmpl, struct rtattr **tb)
 static struct crypto_template hmac_tmpl = {
 	.name = "hmac",
 	.create = hmac_create,
-	.free = shash_free_instance,
 	.module = THIS_MODULE,
 };
 
diff --git a/crypto/shash.c b/crypto/shash.c
index 2f6adb49727b..e05e75b0f402 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -590,12 +590,12 @@ int shash_register_instance(struct crypto_template *tmpl,
 }
 EXPORT_SYMBOL_GPL(shash_register_instance);
 
-void shash_free_instance(struct crypto_instance *inst)
+void shash_free_singlespawn_instance(struct shash_instance *inst)
 {
-	crypto_drop_spawn(crypto_instance_ctx(inst));
-	kfree(shash_instance(inst));
+	crypto_drop_spawn(shash_instance_ctx(inst));
+	kfree(inst);
 }
-EXPORT_SYMBOL_GPL(shash_free_instance);
+EXPORT_SYMBOL_GPL(shash_free_singlespawn_instance);
 
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Synchronous cryptographic hash type");
diff --git a/crypto/vmac.c b/crypto/vmac.c
index 3841b6e46081..fa8c4a7560a9 100644
--- a/crypto/vmac.c
+++ b/crypto/vmac.c
@@ -662,10 +662,12 @@ static int vmac_create(struct crypto_template *tmpl, struct rtattr **tb)
 	inst->alg.final = vmac_final;
 	inst->alg.setkey = vmac_setkey;
 
+	inst->free = shash_free_singlespawn_instance;
+
 	err = shash_register_instance(tmpl, inst);
 	if (err) {
 err_free_inst:
-		shash_free_instance(shash_crypto_instance(inst));
+		shash_free_singlespawn_instance(inst);
 	}
 	return err;
 }
@@ -673,7 +675,6 @@ static int vmac_create(struct crypto_template *tmpl, struct rtattr **tb)
 static struct crypto_template vmac64_tmpl = {
 	.name = "vmac64",
 	.create = vmac_create,
-	.free = shash_free_instance,
 	.module = THIS_MODULE,
 };
 
diff --git a/crypto/xcbc.c b/crypto/xcbc.c
index 9265e00ea663..598ec88abf0f 100644
--- a/crypto/xcbc.c
+++ b/crypto/xcbc.c
@@ -239,10 +239,12 @@ static int xcbc_create(struct crypto_template *tmpl, struct rtattr **tb)
 	inst->alg.final = crypto_xcbc_digest_final;
 	inst->alg.setkey = crypto_xcbc_digest_setkey;
 
+	inst->free = shash_free_singlespawn_instance;
+
 	err = shash_register_instance(tmpl, inst);
 	if (err) {
 err_free_inst:
-		shash_free_instance(shash_crypto_instance(inst));
+		shash_free_singlespawn_instance(inst);
 	}
 	return err;
 }
@@ -250,7 +252,6 @@ static int xcbc_create(struct crypto_template *tmpl, struct rtattr **tb)
 static struct crypto_template crypto_xcbc_tmpl = {
 	.name = "xcbc",
 	.create = xcbc_create,
-	.free = shash_free_instance,
 	.module = THIS_MODULE,
 };
 
diff --git a/include/crypto/internal/hash.h b/include/crypto/internal/hash.h
index c550386221bb..89f6f46ab2b8 100644
--- a/include/crypto/internal/hash.h
+++ b/include/crypto/internal/hash.h
@@ -125,7 +125,7 @@ int crypto_register_shashes(struct shash_alg *algs, int count);
 void crypto_unregister_shashes(struct shash_alg *algs, int count);
 int shash_register_instance(struct crypto_template *tmpl,
 			    struct shash_instance *inst);
-void shash_free_instance(struct crypto_instance *inst);
+void shash_free_singlespawn_instance(struct shash_instance *inst);
 
 int crypto_grab_shash(struct crypto_shash_spawn *spawn,
 		      struct crypto_instance *inst,
-- 
2.24.1

[PATCH v2 5/6] crypto: algapi - remove crypto_template::{alloc,free}()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Now that all templates provide a ->create() method which creates an
instance, installs a strongly-typed ->free() method directly to it, and
registers it, the older ->alloc() and ->free() methods in
'struct crypto_template' are no longer used.  Remove them.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/aead.c           |  5 -----
 crypto/ahash.c          |  5 -----
 crypto/algapi.c         |  5 -----
 crypto/algboss.c        | 12 +-----------
 crypto/shash.c          |  5 -----
 include/crypto/algapi.h |  2 --
 6 files changed, 1 insertion(+), 33 deletions(-)

diff --git a/crypto/aead.c b/crypto/aead.c
index 02a0db076d7e..7707d3223101 100644
--- a/crypto/aead.c
+++ b/crypto/aead.c
@@ -185,11 +185,6 @@ static void crypto_aead_free_instance(struct crypto_instance *inst)
 {
 	struct aead_instance *aead = aead_instance(inst);
 
-	if (!aead->free) {
-		inst->tmpl->free(inst);
-		return;
-	}
-
 	aead->free(aead);
 }
 
diff --git a/crypto/ahash.c b/crypto/ahash.c
index 61e374d76b04..cd5d9847d513 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -515,11 +515,6 @@ static void crypto_ahash_free_instance(struct crypto_instance *inst)
 {
 	struct ahash_instance *ahash = ahash_instance(inst);
 
-	if (!ahash->free) {
-		inst->tmpl->free(inst);
-		return;
-	}
-
 	ahash->free(ahash);
 }
 
diff --git a/crypto/algapi.c b/crypto/algapi.c
index 72592795c7e7..69605e21af92 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -65,11 +65,6 @@ static int crypto_check_alg(struct crypto_alg *alg)
 
 static void crypto_free_instance(struct crypto_instance *inst)
 {
-	if (!inst->alg.cra_type->free) {
-		inst->tmpl->free(inst);
-		return;
-	}
-
 	inst->alg.cra_type->free(inst);
 }
 
diff --git a/crypto/algboss.c b/crypto/algboss.c
index a62149d6c839..535f1f87e6c1 100644
--- a/crypto/algboss.c
+++ b/crypto/algboss.c
@@ -58,7 +58,6 @@ static int cryptomgr_probe(void *data)
 {
 	struct cryptomgr_param *param = data;
 	struct crypto_template *tmpl;
-	struct crypto_instance *inst;
 	int err;
 
 	tmpl = crypto_lookup_template(param->template);
@@ -66,16 +65,7 @@ static int cryptomgr_probe(void *data)
 		goto out;
 
 	do {
-		if (tmpl->create) {
-			err = tmpl->create(tmpl, param->tb);
-			continue;
-		}
-
-		inst = tmpl->alloc(param->tb);
-		if (IS_ERR(inst))
-			err = PTR_ERR(inst);
-		else if ((err = crypto_register_instance(tmpl, inst)))
-			tmpl->free(inst);
+		err = tmpl->create(tmpl, param->tb);
 	} while (err == -EAGAIN && !signal_pending(current));
 
 	crypto_tmpl_put(tmpl);
diff --git a/crypto/shash.c b/crypto/shash.c
index e05e75b0f402..70faf28b2d14 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -427,11 +427,6 @@ static void crypto_shash_free_instance(struct crypto_instance *inst)
 {
 	struct shash_instance *shash = shash_instance(inst);
 
-	if (!shash->free) {
-		inst->tmpl->free(inst);
-		return;
-	}
-
 	shash->free(shash);
 }
 
diff --git a/include/crypto/algapi.h b/include/crypto/algapi.h
index c16c50f8dac1..e115f9215ed5 100644
--- a/include/crypto/algapi.h
+++ b/include/crypto/algapi.h
@@ -63,8 +63,6 @@ struct crypto_template {
 	struct hlist_head instances;
 	struct module *module;
 
-	struct crypto_instance *(*alloc)(struct rtattr **tb);
-	void (*free)(struct crypto_instance *inst);
 	int (*create)(struct crypto_template *tmpl, struct rtattr **tb);
 
 	char name[CRYPTO_MAX_ALG_NAME];
-- 
2.24.1

[PATCH v2 6/6] crypto: algapi - enforce that all instances have a ->free() method

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

All instances need to have a ->free() method, but people could forget to
set it and then not notice if the instance is never unregistered.  To
help detect this bug earlier, don't allow an instance without a ->free()
method to be registered, and complain loudly if someone tries to do it.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/aead.c     | 3 +++
 crypto/ahash.c    | 3 +++
 crypto/akcipher.c | 2 ++
 crypto/shash.c    | 3 +++
 crypto/skcipher.c | 3 +++
 5 files changed, 14 insertions(+)

diff --git a/crypto/aead.c b/crypto/aead.c
index 7707d3223101..16991095270d 100644
--- a/crypto/aead.c
+++ b/crypto/aead.c
@@ -288,6 +288,9 @@ int aead_register_instance(struct crypto_template *tmpl,
 {
 	int err;
 
+	if (WARN_ON(!inst->free))
+		return -EINVAL;
+
 	err = aead_prepare_alg(&inst->alg);
 	if (err)
 		return err;
diff --git a/crypto/ahash.c b/crypto/ahash.c
index cd5d9847d513..68a0f0cb75c4 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -656,6 +656,9 @@ int ahash_register_instance(struct crypto_template *tmpl,
 {
 	int err;
 
+	if (WARN_ON(!inst->free))
+		return -EINVAL;
+
 	err = ahash_prepare_alg(&inst->alg);
 	if (err)
 		return err;
diff --git a/crypto/akcipher.c b/crypto/akcipher.c
index eeed6c151d2f..f866085c8a4a 100644
--- a/crypto/akcipher.c
+++ b/crypto/akcipher.c
@@ -147,6 +147,8 @@ EXPORT_SYMBOL_GPL(crypto_unregister_akcipher);
 int akcipher_register_instance(struct crypto_template *tmpl,
 			       struct akcipher_instance *inst)
 {
+	if (WARN_ON(!inst->free))
+		return -EINVAL;
 	akcipher_prepare_alg(&inst->alg);
 	return crypto_register_instance(tmpl, akcipher_crypto_instance(inst));
 }
diff --git a/crypto/shash.c b/crypto/shash.c
index 70faf28b2d14..c075b26c2a1d 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -577,6 +577,9 @@ int shash_register_instance(struct crypto_template *tmpl,
 {
 	int err;
 
+	if (WARN_ON(!inst->free))
+		return -EINVAL;
+
 	err = shash_prepare_alg(&inst->alg);
 	if (err)
 		return err;
diff --git a/crypto/skcipher.c b/crypto/skcipher.c
index 8c37243307aa..ba41f81fac0b 100644
--- a/crypto/skcipher.c
+++ b/crypto/skcipher.c
@@ -876,6 +876,9 @@ int skcipher_register_instance(struct crypto_template *tmpl,
 {
 	int err;
 
+	if (WARN_ON(!inst->free))
+		return -EINVAL;
+
 	err = skcipher_prepare_alg(&inst->alg);
 	if (err)
 		return err;
-- 
2.24.1

Re: [PATCH v2 0/6] crypto: remove old way of allocating and freeing instances

[Herbert Xu]

Eric Biggers <ebiggers@kernel.org> wrote:
> This series makes all crypto templates use the new way of freeing
> instances where a ->free() method is installed to the instance struct
> itself.  This replaces the weakly-typed method crypto_template::free().
> 
> skcipher and akcipher were already using the new way, while aead was
> mostly but not always using the new way.  shash and ahash were using the
> old way.  This series eliminates this inconsistency (and the redundant
> code associated with it) by making everyone use the new way.
> 
> The last patch adds registration-time checks which verify that all
> instances really have a ->free() method.
> 
> This series is an internal cleanup only; there are no changes for users
> of the crypto API.
> 
> This series is based on top of my other series
> "[PATCH v2 00/28] crypto: template instantiation cleanup".
> 
> Changed v1 => v2:
> 
>  - Rebased onto v2 of the other series.
> 
> Eric Biggers (6):
>  crypto: hash - add support for new way of freeing instances
>  crypto: geniv - convert to new way of freeing instances
>  crypto: cryptd - convert to new way of freeing instances
>  crypto: shash - convert shash_free_instance() to new style
>  crypto: algapi - remove crypto_template::{alloc,free}()
>  crypto: algapi - enforce that all instances have a ->free() method
> 
> crypto/aead.c                   |  8 +++----
> crypto/ahash.c                  | 11 +++++++++
> crypto/akcipher.c               |  2 ++
> crypto/algapi.c                 |  5 ----
> crypto/algboss.c                | 12 +---------
> crypto/ccm.c                    |  5 ++--
> crypto/cmac.c                   |  5 ++--
> crypto/cryptd.c                 | 42 ++++++++++++++++-----------------
> crypto/echainiv.c               | 20 ++++------------
> crypto/geniv.c                  | 15 ++++++------
> crypto/hmac.c                   |  5 ++--
> crypto/seqiv.c                  | 20 ++++------------
> crypto/shash.c                  | 19 +++++++++++----
> crypto/skcipher.c               |  3 +++
> crypto/vmac.c                   |  5 ++--
> crypto/xcbc.c                   |  5 ++--
> include/crypto/algapi.h         |  2 --
> include/crypto/internal/geniv.h |  1 -
> include/crypto/internal/hash.h  |  4 +++-
> 19 files changed, 89 insertions(+), 100 deletions(-)

All 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