pcrypt: handle crypto_get_attr_type() errors

PATCH Superseded Dan Carpenter 3 participants 8 messages Mar 21, 2010 – Mar 24, 2010

Subsystems (3)

Kernel janitors Crypto API Pcrypt parallel crypto engine

lore

Share & tools

lore

Dan Carpenter

author

2010-03-21 09:28 UTC #0 lore raw

PATCH 1/1 pcrypt: handle crypto_get_attr_type() errors

crypto_get_attr_type() can returns ERR_PTRs if there is a problem.

Signed-off-by Dan Carpenter <[email protected]>

crypto/pcrypt.c +2 -0

index 8020124..41bd80f 100644
@@ -322,6 +322,8 @@ static struct crypto_instance *pcrypt_alloc_aead(struct rtattr **tb)
 	struct crypto_attr_type *algt;
 
 	algt = crypto_get_attr_type(tb);
+	if (IS_ERR(algt))
+		return ERR_CAST(algt);
 
 	alg = crypto_get_attr_alg(tb, algt->type,
 				  (algt->mask & CRYPTO_ALG_TYPE_MASK));

Steffen Klassert

maintainer

2010-03-22 13:21 UTC #1 lore raw

Quoted 9 lines · Dan Carpenter

crypto_get_attr_type() can returns ERR_PTRs if there is a problem.
Signed-off-by Dan Carpenter <[email protected]>
crypto/pcrypt.c +0 -0
index 8020124..41bd80f 100644

crypto/pcrypt.c static struct crypto_instance *pcrypt_alloc_aead(struct rtattr **tb) · line 329

Quoted 8 lines

crypto/pcrypt.c +2 -0

@@ -322,6 +322,8 @@ static struct crypto_instance *pcrypt_alloc_aead(struct rtattr **tb)
 	struct crypto_attr_type *algt;
 
 	algt = crypto_get_attr_type(tb);
+	if (IS_ERR(algt))
+		return ERR_CAST(algt);
 
 	alg = crypto_get_attr_alg(tb, algt->type,
 				  (algt->mask & CRYPTO_ALG_TYPE_MASK));

I've just noticed that we are calling crypto_get_attr_type already in pcrypt_alloc, so perhaps we could just pass the type and mask to pcrypt_alloc_aead. Then we can remove this second call to crypto_get_attr_type completely.

Steffen

Dan Carpenter

author

2010-03-22 13:48 UTC #2 lore raw

Quoted 16 lines · Steffen Klassert

Quoted 15 lines · Dan Carpenter
crypto_get_attr_type() can returns ERR_PTRs if there is a problem.
Signed-off-by Dan Carpenter <[email protected]>
crypto/pcrypt.c +2 -0
index 8020124..41bd80f 100644
@@ -322,6 +322,8 @@ static struct crypto_instance *pcrypt_alloc_aead(struct rtattr **tb)
 	struct crypto_attr_type *algt;
 
 	algt = crypto_get_attr_type(tb);
+	if (IS_ERR(algt))
+		return ERR_CAST(algt);
 

Quoted 8 lines

crypto/pcrypt.c static struct crypto_instance *pcrypt_alloc_aead(struct rtattr **tb) · line 329
Quoted 2 lines
crypto/pcrypt.c +0 -0
 	alg = crypto_get_attr_alg(tb, algt->type,
 				  (algt->mask & CRYPTO_ALG_TYPE_MASK));
I've just noticed that we are calling crypto_get_attr_type already in pcrypt_alloc, so perhaps we could just pass the type and mask to pcrypt_alloc_aead. Then we can remove this second call to crypto_get_attr_type completely.

Yup. That works too. I will send an updated patch.

regards,
dan carpenter

Quoted 1 line

Steffen

Dan Carpenter

author

2010-03-22 13:53 UTC #3 lore raw

PATCH v2 1/1 pcrypt: handle crypto_get_attr_type() errors

In reply to Steffen Klassert

I was concerned about the error handling for crypto_get_attr_type() in pcrypt_alloc_aead(). Steffen Klassert pointed out that we could simply avoid calling crypto_get_attr_type() if we passed the type and mask as a parameter.

Signed-off-by Dan Carpenter <[email protected]>

crypto/pcrypt.c +3 -5

index 8020124..e049388 100644
@@ -315,13 +315,11 @@ out_free_inst:
 	goto out;
 }
 
-static struct crypto_instance *pcrypt_alloc_aead(struct rtattr **tb)
+static struct crypto_instance *pcrypt_alloc_aead(struct rtattr **tb,
+						 struct crypto_attr_type *algt)
 {
 	struct crypto_instance *inst;
 	struct crypto_alg *alg;
-	struct crypto_attr_type *algt;
-
-	algt = crypto_get_attr_type(tb);
 
 	alg = crypto_get_attr_alg(tb, algt->type,
 				  (algt->mask & CRYPTO_ALG_TYPE_MASK));

@@ -365,7 +363,7 @@ static struct crypto_instance *pcrypt_alloc(struct rtattr **tb)
 
 	switch (algt->type & algt->mask & CRYPTO_ALG_TYPE_MASK) {
 	case CRYPTO_ALG_TYPE_AEAD:
-		return pcrypt_alloc_aead(tb);
+		return pcrypt_alloc_aead(tb, algt);
 	}
 
 	return ERR_PTR(-EINVAL);

Steffen Klassert

maintainer

2010-03-22 14:51 UTC #4 lore raw

Quoted 14 lines · Dan Carpenter

crypto/pcrypt.c +2 -4

-static struct crypto_instance *pcrypt_alloc_aead(struct rtattr **tb)
+static struct crypto_instance *pcrypt_alloc_aead(struct rtattr **tb,
+						 struct crypto_attr_type *algt)
 {
 	struct crypto_instance *inst;
 	struct crypto_alg *alg;
-	struct crypto_attr_type *algt;
-
-	algt = crypto_get_attr_type(tb);
 
 	alg = crypto_get_attr_alg(tb, algt->type,
 				  (algt->mask & CRYPTO_ALG_TYPE_MASK));

crypto/pcrypt.c static struct crypto_instance *pcrypt_alloc(struct rtattr **tb) · line 368

Quoted 7 lines

crypto/pcrypt.c +1 -1

@@ -365,7 +363,7 @@ static struct crypto_instance *pcrypt_alloc(struct rtattr **tb)
 
 	switch (algt->type & algt->mask & CRYPTO_ALG_TYPE_MASK) {
 	case CRYPTO_ALG_TYPE_AEAD:
-		return pcrypt_alloc_aead(tb);
+		return pcrypt_alloc_aead(tb, algt);
 	}

I thought about passing the type and mask values separately to pcrypt_alloc_aead, like type and mask values are passed to crypto_get_attr_alg. This is the usual way to do this in the crypto layer.

Thanks,

Steffen

Dan Carpenter

author

2010-03-22 15:28 UTC #5 lore raw

PATCH v3 1/1 pcrypt: handle crypto_get_attr_type() errors

Signed-off-by Dan Carpenter <[email protected]>

---
All three versions have basically been the same except for style issues. I will confess that this (hopefully final :P) version looks much nicer than the earlier ones.

crypto/pcrypt.c +4 -7

index 8020124..247178c 100644
@@ -315,16 +315,13 @@ out_free_inst:
 	goto out;
 }
 
-static struct crypto_instance *pcrypt_alloc_aead(struct rtattr **tb)
+static struct crypto_instance *pcrypt_alloc_aead(struct rtattr **tb,
+						 u32 type, u32 mask)
 {
 	struct crypto_instance *inst;
 	struct crypto_alg *alg;
-	struct crypto_attr_type *algt;
-
-	algt = crypto_get_attr_type(tb);
 
-	alg = crypto_get_attr_alg(tb, algt->type,
-				  (algt->mask & CRYPTO_ALG_TYPE_MASK));
+	alg = crypto_get_attr_alg(tb, type, (mask & CRYPTO_ALG_TYPE_MASK));
 	if (IS_ERR(alg))
 		return ERR_CAST(alg);

@@ -365,7 +362,7 @@ static struct crypto_instance *pcrypt_alloc(struct rtattr **tb)
 
 	switch (algt->type & algt->mask & CRYPTO_ALG_TYPE_MASK) {
 	case CRYPTO_ALG_TYPE_AEAD:
-		return pcrypt_alloc_aead(tb);
+		return pcrypt_alloc_aead(tb, algt->type, algt->mask);
 	}
 
 	return ERR_PTR(-EINVAL);

Steffen Klassert

maintainer Acked-by on v3 1/1

2010-03-23 10:32 UTC #6 lore raw

Quoted 6 lines · Dan Carpenter

I was concerned about the error handling for crypto_get_attr_type() in pcrypt_alloc_aead(). Steffen Klassert pointed out that we could simply avoid calling crypto_get_attr_type() if we passed the type and mask as a parameters.
Signed-off-by Dan Carpenter <[email protected]>

Acked-by Steffen Klassert <[email protected]>

Thanks a lot!

Quoted 5 lines

---
All three versions have basically been the same except for style issues. I will confess that this (hopefully final :P) version looks much nicer than the earlier ones.

It's not only style issues. We also got some additional benefit from this version. We got rid of a superfluous call to crypto_get_attr_type and we are able to change type and mask before we pass it to pcrypt_alloc_aead if this becomes necessary.

Steffen

Herbert Xu

maintainer

2010-03-24 13:35 UTC #7 lore raw

�