[PATCH 1/2] crypto: fix crypto_alloc_{tfm,base}() return value

[PATCH 1/2] crypto: fix crypto_alloc_{tfm,base}() return value

[Akinobu Mita]

crypto_alloc_tfm() and crypto_alloc_base() are suppose to return error code
as pointer on failures. But there are some cases where they return NULL
(for example, crypto_alloc_tfm() returns NULL on kzalloc() failure)

This patch fixes that wrong return value, and also fixes tcrypt so that it can
detect error code correctly.

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>

 crypto/api.c    |   47 ++++++++++++++++++++++++++++-------------------
 crypto/tcrypt.c |    5 +++--
 2 files changed, 31 insertions(+), 21 deletions(-)

Index: work-fault-inject/crypto/tcrypt.c
===================================================================
--- work-fault-inject.orig/crypto/tcrypt.c	2006-10-09 15:06:37.000000000 +0900
+++ work-fault-inject/crypto/tcrypt.c	2006-10-09 15:09:00.000000000 +0900
@@ -766,8 +766,9 @@ static void test_deflate(void)
 	tv = (void *)tvmem;
 
 	tfm = crypto_alloc_tfm("deflate", 0);
-	if (tfm == NULL) {
-		printk("failed to load transform for deflate\n");
+	if (IS_ERR(tfm)) {
+		printk("failed to load transform for deflate: %ld\n",
+				PTR_ERR(tfm));
 		return;
 	}
 
Index: work-fault-inject/crypto/api.c
===================================================================
--- work-fault-inject.orig/crypto/api.c	2006-10-09 15:06:37.000000000 +0900
+++ work-fault-inject/crypto/api.c	2006-10-09 15:09:00.000000000 +0900
@@ -331,7 +331,7 @@ struct crypto_tfm *__crypto_alloc_tfm(st
 	tfm_size = sizeof(*tfm) + crypto_ctxsize(alg, flags);
 	tfm = kzalloc(tfm_size, GFP_KERNEL);
 	if (tfm == NULL)
-		goto out;
+		goto out_err;
 
 	tfm->__crt_alg = alg;
 
@@ -355,6 +355,7 @@ cra_init_failed:
 	crypto_exit_ops(tfm);
 out_free_tfm:
 	kfree(tfm);
+out_err:
 	tfm = ERR_PTR(err);
 out:
 	return tfm;
@@ -363,27 +364,35 @@ EXPORT_SYMBOL_GPL(__crypto_alloc_tfm);
 
 struct crypto_tfm *crypto_alloc_tfm(const char *name, u32 flags)
 {
-	struct crypto_tfm *tfm = NULL;
+	struct crypto_tfm *tfm;
 	int err;
 
-	do {
+	for (;;) {
 		struct crypto_alg *alg;
 
 		alg = crypto_alg_mod_lookup(name, 0, CRYPTO_ALG_ASYNC);
-		err = PTR_ERR(alg);
-		if (IS_ERR(alg))
-			continue;
+		if (IS_ERR(alg)) {
+			err = PTR_ERR(alg);
+			goto err;
+		}
 
 		tfm = __crypto_alloc_tfm(alg, flags);
-		err = 0;
-		if (IS_ERR(tfm)) {
-			crypto_mod_put(alg);
-			err = PTR_ERR(tfm);
-			tfm = NULL;
+		if (!IS_ERR(tfm))
+			return tfm;
+
+		crypto_mod_put(alg);
+		err = PTR_ERR(tfm);
+
+err:
+		if (err != -EAGAIN)
+			break;
+		if (signal_pending(current)) {
+			err = -EINTR;
+			break;
 		}
-	} while (err == -EAGAIN && !signal_pending(current));
+	}
 
-	return tfm;
+	return ERR_PTR(err);
 }
 
 /*
@@ -414,14 +423,14 @@ struct crypto_tfm *crypto_alloc_base(con
 		struct crypto_alg *alg;
 
 		alg = crypto_alg_mod_lookup(alg_name, type, mask);
-		err = PTR_ERR(alg);
-		tfm = ERR_PTR(err);
-		if (IS_ERR(alg))
+		if (IS_ERR(alg)) {
+			err = PTR_ERR(alg);
 			goto err;
+		}
 
 		tfm = __crypto_alloc_tfm(alg, 0);
 		if (!IS_ERR(tfm))
-			break;
+			return tfm;
 
 		crypto_mod_put(alg);
 		err = PTR_ERR(tfm);
@@ -433,9 +442,9 @@ err:
 			err = -EINTR;
 			break;
 		}
-	};
+	}
 
-	return tfm;
+	return ERR_PTR(err);
 }
 EXPORT_SYMBOL_GPL(crypto_alloc_base);
  

[PATCH 2/2] crypto: delete duplicated code

[Akinobu Mita]

crypto_alloc_tfm() and crypto_alloc_base() are almost same.

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>

 crypto/api.c |   40 +++++++++-------------------------------
 1 file changed, 9 insertions(+), 31 deletions(-)

Index: work-fault-inject/crypto/api.c
===================================================================
--- work-fault-inject.orig/crypto/api.c	2006-10-09 15:09:00.000000000 +0900
+++ work-fault-inject/crypto/api.c	2006-10-09 15:09:04.000000000 +0900
@@ -362,7 +362,8 @@ out:
 }
 EXPORT_SYMBOL_GPL(__crypto_alloc_tfm);
 
-struct crypto_tfm *crypto_alloc_tfm(const char *name, u32 flags)
+static struct crypto_tfm *crypto_alloc_tfm_base(const char *name, u32 flags,
+						u32 type, u32 mask)
 {
 	struct crypto_tfm *tfm;
 	int err;
@@ -370,7 +371,7 @@ struct crypto_tfm *crypto_alloc_tfm(cons
 	for (;;) {
 		struct crypto_alg *alg;
 
-		alg = crypto_alg_mod_lookup(name, 0, CRYPTO_ALG_ASYNC);
+		alg = crypto_alg_mod_lookup(name, type, mask);
 		if (IS_ERR(alg)) {
 			err = PTR_ERR(alg);
 			goto err;
@@ -395,6 +396,11 @@ err:
 	return ERR_PTR(err);
 }
 
+struct crypto_tfm *crypto_alloc_tfm(const char *name, u32 flags)
+{
+	return crypto_alloc_tfm_base(name, flags, 0, CRYPTO_ALG_ASYNC);
+}
+
 /*
  *	crypto_alloc_base - Locate algorithm and allocate transform
  *	@alg_name: Name of algorithm
@@ -416,35 +422,7 @@ err:
  */
 struct crypto_tfm *crypto_alloc_base(const char *alg_name, u32 type, u32 mask)
 {
-	struct crypto_tfm *tfm;
-	int err;
-
-	for (;;) {
-		struct crypto_alg *alg;
-
-		alg = crypto_alg_mod_lookup(alg_name, type, mask);
-		if (IS_ERR(alg)) {
-			err = PTR_ERR(alg);
-			goto err;
-		}
-
-		tfm = __crypto_alloc_tfm(alg, 0);
-		if (!IS_ERR(tfm))
-			return tfm;
-
-		crypto_mod_put(alg);
-		err = PTR_ERR(tfm);
-
-err:
-		if (err != -EAGAIN)
-			break;
-		if (signal_pending(current)) {
-			err = -EINTR;
-			break;
-		}
-	}
-
-	return ERR_PTR(err);
+	return crypto_alloc_tfm_base(alg_name, 0, type, mask);
 }
 EXPORT_SYMBOL_GPL(crypto_alloc_base);
  

Re: [PATCH 1/2] crypto: fix crypto_alloc_{tfm,base}() return value

[Herbert Xu]

On Mon, Oct 09, 2006 at 05:58:12PM +0900, Akinobu Mita wrote:
> crypto_alloc_tfm() and crypto_alloc_base() are suppose to return error code
> as pointer on failures. But there are some cases where they return NULL
> (for example, crypto_alloc_tfm() returns NULL on kzalloc() failure)
> 
> This patch fixes that wrong return value, and also fixes tcrypt so that it can
> detect error code correctly.

Actually, crypto_alloc_tfm is an obsolete function which is supposed
to maintain its previous semantics of returning NULL or success.

I don't quite see where the problem with crypto_alloc_base is.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 1/2] crypto: fix crypto_alloc_{tfm,base}() return value

[Akinobu Mita]

On Mon, Oct 09, 2006 at 09:14:46PM +1000, Herbert Xu wrote:

> Actually, crypto_alloc_tfm is an obsolete function which is supposed
> to maintain its previous semantics of returning NULL or success.

I misunderstood about crypto_alloc_tfm().

BTW, ecryptfs and reiser4 are still using crypto_alloc_tfm().
Should we mark it as __deprecated?

> I don't quite see where the problem with crypto_alloc_base is.

- __crypto_alloc_tfm() should return -ENOMEM on kzalloc() failure.
  But it returns NULL.

- crypto_alloc_base() may not return -EINTR on signal_pending()

I'll fix the patch and resend with more clear description later.

Re: [PATCH 1/2] crypto: fix crypto_alloc_{tfm,base}() return value

[Herbert Xu]

On Mon, Oct 09, 2006 at 08:55:26PM +0900, Akinobu Mita wrote:
> 
> I misunderstood about crypto_alloc_tfm().
> 
> BTW, ecryptfs and reiser4 are still using crypto_alloc_tfm().
> Should we mark it as __deprecated?

Probably.  Although anybody using crypto_alloc_tfm will probably
also use the old crypto interface which gives plenty of warnings
already.

> - __crypto_alloc_tfm() should return -ENOMEM on kzalloc() failure.
>   But it returns NULL.
> 
> - crypto_alloc_base() may not return -EINTR on signal_pending()
> 
> I'll fix the patch and resend with more clear description later.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt