diff mbox series

KEYS: asymmetric: Fix error codes

Message ID c5e34c6a-da1e-4585-98c4-14701b0e093e@moroto.mountain (mailing list archive)
State Accepted
Delegated to: Herbert Xu
Headers show
Series KEYS: asymmetric: Fix error codes | expand

Commit Message

Dan Carpenter July 3, 2023, 2:18 p.m. UTC
These error paths should return the appropriate error codes instead of
returning success.

Fixes: 63ba4d67594a ("KEYS: asymmetric: Use new crypto interface without scatterlists")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 crypto/asymmetric_keys/public_key.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

Comments

Herbert Xu July 7, 2023, 4:20 a.m. UTC | #1
On Mon, Jul 03, 2023 at 05:18:08PM +0300, Dan Carpenter wrote:
> These error paths should return the appropriate error codes instead of
> returning success.
> 
> Fixes: 63ba4d67594a ("KEYS: asymmetric: Use new crypto interface without scatterlists")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  crypto/asymmetric_keys/public_key.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)

Patch applied.  Thanks.
David Howells July 7, 2023, 6:01 a.m. UTC | #2
Dan Carpenter <dan.carpenter@linaro.org> wrote:

> These error paths should return the appropriate error codes instead of
> returning success.
> 
> Fixes: 63ba4d67594a ("KEYS: asymmetric: Use new crypto interface without scatterlists")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>

Reviewed-by: David Howells <dhowells@redhat.com>
Jarkko Sakkinen July 10, 2023, 11:10 p.m. UTC | #3
On Mon Jul 3, 2023 at 5:18 PM EEST, Dan Carpenter wrote:
> These error paths should return the appropriate error codes instead of
> returning success.
>
> Fixes: 63ba4d67594a ("KEYS: asymmetric: Use new crypto interface without scatterlists")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  crypto/asymmetric_keys/public_key.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
> index e787598cb3f7..773e159dbbcb 100644
> --- a/crypto/asymmetric_keys/public_key.c
> +++ b/crypto/asymmetric_keys/public_key.c
> @@ -185,8 +185,10 @@ static int software_key_query(const struct kernel_pkey_params *params,
>  
>  	if (issig) {
>  		sig = crypto_alloc_sig(alg_name, 0, 0);
> -		if (IS_ERR(sig))
> +		if (IS_ERR(sig)) {
> +			ret = PTR_ERR(sig);
>  			goto error_free_key;
> +		}
>  
>  		if (pkey->key_is_private)
>  			ret = crypto_sig_set_privkey(sig, key, pkey->keylen);
> @@ -208,8 +210,10 @@ static int software_key_query(const struct kernel_pkey_params *params,
>  		}
>  	} else {
>  		tfm = crypto_alloc_akcipher(alg_name, 0, 0);
> -		if (IS_ERR(tfm))
> +		if (IS_ERR(tfm)) {
> +			ret = PTR_ERR(tfm);
>  			goto error_free_key;
> +		}
>  
>  		if (pkey->key_is_private)
>  			ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen);
> @@ -300,8 +304,10 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
>  
>  	if (issig) {
>  		sig = crypto_alloc_sig(alg_name, 0, 0);
> -		if (IS_ERR(sig))
> +		if (IS_ERR(sig)) {
> +			ret = PTR_ERR(sig);
>  			goto error_free_key;
> +		}
>  
>  		if (pkey->key_is_private)
>  			ret = crypto_sig_set_privkey(sig, key, pkey->keylen);
> @@ -313,8 +319,10 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
>  		ksz = crypto_sig_maxsize(sig);
>  	} else {
>  		tfm = crypto_alloc_akcipher(alg_name, 0, 0);
> -		if (IS_ERR(tfm))
> +		if (IS_ERR(tfm)) {
> +			ret = PTR_ERR(tfm);
>  			goto error_free_key;
> +		}
>  
>  		if (pkey->key_is_private)
>  			ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen);
> @@ -411,8 +419,10 @@ int public_key_verify_signature(const struct public_key *pkey,
>  
>  	key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen,
>  		      GFP_KERNEL);
> -	if (!key)
> +	if (!key) {
> +		ret = -ENOMEM;
>  		goto error_free_tfm;
> +	}
>  
>  	memcpy(key, pkey->key, pkey->keylen);
>  	ptr = key + pkey->keylen;
> -- 
> 2.39.2

I'll pick this as I'm late with 6.5 PR.

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko
Jarkko Sakkinen July 10, 2023, 11:12 p.m. UTC | #4
On Tue Jul 11, 2023 at 2:10 AM EEST, Jarkko Sakkinen wrote:
> On Mon Jul 3, 2023 at 5:18 PM EEST, Dan Carpenter wrote:
> > These error paths should return the appropriate error codes instead of
> > returning success.
> >
> > Fixes: 63ba4d67594a ("KEYS: asymmetric: Use new crypto interface without scatterlists")
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> >  crypto/asymmetric_keys/public_key.c | 20 +++++++++++++++-----
> >  1 file changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
> > index e787598cb3f7..773e159dbbcb 100644
> > --- a/crypto/asymmetric_keys/public_key.c
> > +++ b/crypto/asymmetric_keys/public_key.c
> > @@ -185,8 +185,10 @@ static int software_key_query(const struct kernel_pkey_params *params,
> >  
> >  	if (issig) {
> >  		sig = crypto_alloc_sig(alg_name, 0, 0);
> > -		if (IS_ERR(sig))
> > +		if (IS_ERR(sig)) {
> > +			ret = PTR_ERR(sig);
> >  			goto error_free_key;
> > +		}
> >  
> >  		if (pkey->key_is_private)
> >  			ret = crypto_sig_set_privkey(sig, key, pkey->keylen);
> > @@ -208,8 +210,10 @@ static int software_key_query(const struct kernel_pkey_params *params,
> >  		}
> >  	} else {
> >  		tfm = crypto_alloc_akcipher(alg_name, 0, 0);
> > -		if (IS_ERR(tfm))
> > +		if (IS_ERR(tfm)) {
> > +			ret = PTR_ERR(tfm);
> >  			goto error_free_key;
> > +		}
> >  
> >  		if (pkey->key_is_private)
> >  			ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen);
> > @@ -300,8 +304,10 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
> >  
> >  	if (issig) {
> >  		sig = crypto_alloc_sig(alg_name, 0, 0);
> > -		if (IS_ERR(sig))
> > +		if (IS_ERR(sig)) {
> > +			ret = PTR_ERR(sig);
> >  			goto error_free_key;
> > +		}
> >  
> >  		if (pkey->key_is_private)
> >  			ret = crypto_sig_set_privkey(sig, key, pkey->keylen);
> > @@ -313,8 +319,10 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
> >  		ksz = crypto_sig_maxsize(sig);
> >  	} else {
> >  		tfm = crypto_alloc_akcipher(alg_name, 0, 0);
> > -		if (IS_ERR(tfm))
> > +		if (IS_ERR(tfm)) {
> > +			ret = PTR_ERR(tfm);
> >  			goto error_free_key;
> > +		}
> >  
> >  		if (pkey->key_is_private)
> >  			ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen);
> > @@ -411,8 +419,10 @@ int public_key_verify_signature(const struct public_key *pkey,
> >  
> >  	key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen,
> >  		      GFP_KERNEL);
> > -	if (!key)
> > +	if (!key) {
> > +		ret = -ENOMEM;
> >  		goto error_free_tfm;
> > +	}
> >  
> >  	memcpy(key, pkey->key, pkey->keylen);
> >  	ptr = key + pkey->keylen;
> > -- 
> > 2.39.2
>
> I'll pick this as I'm late with 6.5 PR.
>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Causes merge conflicts with my tree:

https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/

BR, Jarkko
Dan Carpenter July 11, 2023, 8:40 a.m. UTC | #5
On Tue, Jul 11, 2023 at 02:12:22AM +0300, Jarkko Sakkinen wrote:
> > > Fixes: 63ba4d67594a ("KEYS: asymmetric: Use new crypto interface without scatterlists")

[ snip ]

> >
> > I'll pick this as I'm late with 6.5 PR.
> >
> > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> 
> Causes merge conflicts with my tree:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/

Your master branch doesn't include the "Use new crypto interface" commit
so it doesn't have the bug.

(I'm just testing against linux-next and I don't know how the crypto
trees work).

regards,
dan carpenter
Jarkko Sakkinen July 11, 2023, 9:51 p.m. UTC | #6
On Tue, 2023-07-11 at 11:40 +0300, Dan Carpenter wrote:
> On Tue, Jul 11, 2023 at 02:12:22AM +0300, Jarkko Sakkinen wrote:
> > > > Fixes: 63ba4d67594a ("KEYS: asymmetric: Use new crypto interface without scatterlists")
> 
> [ snip ]
> 
> > > 
> > > I'll pick this as I'm late with 6.5 PR.
> > > 
> > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > 
> > Causes merge conflicts with my tree:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/
> 
> Your master branch doesn't include the "Use new crypto interface" commit
> so it doesn't have the bug.
> 
> (I'm just testing against linux-next and I don't know how the crypto
> trees work).
> 
> regards,
> dan carpenter

It is unfortunately based on Linus' tree :-/

BR, Jarkko
Herbert Xu July 11, 2023, 11:27 p.m. UTC | #7
On Wed, Jul 12, 2023 at 12:51:45AM +0300, Jarkko Sakkinen wrote:
> On Tue, 2023-07-11 at 11:40 +0300, Dan Carpenter wrote:
> > On Tue, Jul 11, 2023 at 02:12:22AM +0300, Jarkko Sakkinen wrote:
> > > > > Fixes: 63ba4d67594a ("KEYS: asymmetric: Use new crypto interface without scatterlists")
> > 
> > [ snip ]
> > 
> > > > 
> > > > I'll pick this as I'm late with 6.5 PR.
> > > > 
> > > > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
> > > 
> > > Causes merge conflicts with my tree:
> > > 
> > > https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/
> > 
> > Your master branch doesn't include the "Use new crypto interface" commit
> > so it doesn't have the bug.
> > 
> > (I'm just testing against linux-next and I don't know how the crypto
> > trees work).
> > 
> > regards,
> > dan carpenter
> 
> It is unfortunately based on Linus' tree :-/

Both this fix and the pathces it depends on have gone through
the crypto tree and is now merged upstream.

Thanks,
diff mbox series

Patch

diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index e787598cb3f7..773e159dbbcb 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -185,8 +185,10 @@  static int software_key_query(const struct kernel_pkey_params *params,
 
 	if (issig) {
 		sig = crypto_alloc_sig(alg_name, 0, 0);
-		if (IS_ERR(sig))
+		if (IS_ERR(sig)) {
+			ret = PTR_ERR(sig);
 			goto error_free_key;
+		}
 
 		if (pkey->key_is_private)
 			ret = crypto_sig_set_privkey(sig, key, pkey->keylen);
@@ -208,8 +210,10 @@  static int software_key_query(const struct kernel_pkey_params *params,
 		}
 	} else {
 		tfm = crypto_alloc_akcipher(alg_name, 0, 0);
-		if (IS_ERR(tfm))
+		if (IS_ERR(tfm)) {
+			ret = PTR_ERR(tfm);
 			goto error_free_key;
+		}
 
 		if (pkey->key_is_private)
 			ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen);
@@ -300,8 +304,10 @@  static int software_key_eds_op(struct kernel_pkey_params *params,
 
 	if (issig) {
 		sig = crypto_alloc_sig(alg_name, 0, 0);
-		if (IS_ERR(sig))
+		if (IS_ERR(sig)) {
+			ret = PTR_ERR(sig);
 			goto error_free_key;
+		}
 
 		if (pkey->key_is_private)
 			ret = crypto_sig_set_privkey(sig, key, pkey->keylen);
@@ -313,8 +319,10 @@  static int software_key_eds_op(struct kernel_pkey_params *params,
 		ksz = crypto_sig_maxsize(sig);
 	} else {
 		tfm = crypto_alloc_akcipher(alg_name, 0, 0);
-		if (IS_ERR(tfm))
+		if (IS_ERR(tfm)) {
+			ret = PTR_ERR(tfm);
 			goto error_free_key;
+		}
 
 		if (pkey->key_is_private)
 			ret = crypto_akcipher_set_priv_key(tfm, key, pkey->keylen);
@@ -411,8 +419,10 @@  int public_key_verify_signature(const struct public_key *pkey,
 
 	key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen,
 		      GFP_KERNEL);
-	if (!key)
+	if (!key) {
+		ret = -ENOMEM;
 		goto error_free_tfm;
+	}
 
 	memcpy(key, pkey->key, pkey->keylen);
 	ptr = key + pkey->keylen;