[patch] pcrypt: handle crypto_get_attr_type() errors

[patch] pcrypt: handle crypto_get_attr_type() errors

[Dan Carpenter]

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

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
index 8020124..41bd80f 100644
--- a/crypto/pcrypt.c
+++ b/crypto/pcrypt.c
@@ -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));

Re: [patch] pcrypt: handle crypto_get_attr_type() errors

[Steffen Klassert]

On Sun, Mar 21, 2010 at 12:28:47PM +0300, Dan Carpenter wrote:
> crypto_get_attr_type() can returns ERR_PTRs if there is a problem. 
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
> index 8020124..41bd80f 100644
> --- a/crypto/pcrypt.c
> +++ b/crypto/pcrypt.c
> @@ -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

Re: [patch] pcrypt: handle crypto_get_attr_type() errors

[Dan Carpenter]

On Mon, Mar 22, 2010 at 02:23:42PM +0100, Steffen Klassert wrote:
> On Sun, Mar 21, 2010 at 12:28:47PM +0300, Dan Carpenter wrote:
> > crypto_get_attr_type() can returns ERR_PTRs if there is a problem. 
> > 
> > Signed-off-by: Dan Carpenter <error27@gmail.com>
> > 
> > diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
> > index 8020124..41bd80f 100644
> > --- a/crypto/pcrypt.c
> > +++ b/crypto/pcrypt.c
> > @@ -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.
> 

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

regards,
dan carpenter

> Steffen

[patch v2] pcrypt: handle crypto_get_attr_type() errors

[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 
parameter.

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
index 8020124..e049388 100644
--- a/crypto/pcrypt.c
+++ b/crypto/pcrypt.c
@@ -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);

Re: [patch v2] pcrypt: handle crypto_get_attr_type() errors

[Steffen Klassert]

On Mon, Mar 22, 2010 at 04:53:19PM +0300, Dan Carpenter wrote:
>  
> -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);
>  	}
>  

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

[patch v3] pcrypt: handle crypto_get_attr_type() errors

[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 <error27@gmail.com>
---
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.

diff --git a/crypto/pcrypt.c b/crypto/pcrypt.c
index 8020124..247178c 100644
--- a/crypto/pcrypt.c
+++ b/crypto/pcrypt.c
@@ -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;

Re: [patch v3] pcrypt: handle crypto_get_attr_type() errors

[Steffen Klassert]

On Mon, Mar 22, 2010 at 06:28:45PM +0300, Dan Carpenter wrote:
> 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 <error27@gmail.com>

Acked-by: Steffen Klassert <steffen.klassert@secunet.com>

Thanks a lot!

> ---
> 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

Re: [patch v3] pcrypt: handle crypto_get_attr_type() errors

[Herbert Xu]

On Tue, Mar 23, 2010 at 11:34:31AM +0100, Steffen Klassert wrote:
> On Mon, Mar 22, 2010 at 06:28:45PM +0300, Dan Carpenter wrote:
> > 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 <error27@gmail.com>
> 
> Acked-by: Steffen Klassert <steffen.klassert@secunet.com>

Patch applied.  Thanks everyone!
-- 
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