| Message ID | c5e34c6a-da1e-4585-98c4-14701b0e093e@moroto.mountain |
|---|---|
| State | Accepted |
| Commit | 9e9311e04e63ede92be98425efd843f9836336bd |
| Headers | show |
| Series | KEYS: asymmetric: Fix error codes | expand |
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.
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>
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
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
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
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
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 --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;
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(-)