2023-07-03 14:25:25

by Dan Carpenter

[permalink] [raw]
Subject: [PATCH] KEYS: asymmetric: Fix error codes

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 <[email protected]>
---
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



2023-07-07 04:50:49

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asymmetric: Fix error codes

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 <[email protected]>
> ---
> crypto/asymmetric_keys/public_key.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)

Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2023-07-07 06:05:11

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asymmetric: Fix error codes

Dan Carpenter <[email protected]> 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 <[email protected]>

Reviewed-by: David Howells <[email protected]>


2023-07-10 23:22:09

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asymmetric: Fix error codes

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 <[email protected]>
> > ---
> > 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 <[email protected]>

Causes merge conflicts with my tree:

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

BR, Jarkko

2023-07-10 23:22:09

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asymmetric: Fix error codes

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 <[email protected]>
> ---
> 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 <[email protected]>

BR, Jarkko

2023-07-11 08:54:01

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asymmetric: Fix error codes

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 <[email protected]>
>
> 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

2023-07-11 21:52:54

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asymmetric: Fix error codes

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 <[email protected]>
> >
> > 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


2023-07-11 23:57:27

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] KEYS: asymmetric: Fix error codes

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 <[email protected]>
> > >
> > > 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,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt