| Message ID | 20210112161044.3101-1-toke@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: public_key: check that pkey_algo is non-NULL before passing it to strcmp() | expand |
Hi, I have fixed this problem last week. Still thanks for your fixing. patch is here: https://lkml.org/lkml/2021/1/7/201 Best regards, Tianjia On 1/13/21 12:10 AM, Toke Høiland-Jørgensen wrote: > When public_key_verify_signature() is called from > asymmetric_key_verify_signature(), the pkey_algo field of struct > public_key_signature will be NULL, which causes a NULL pointer dereference > in the strcmp() check. Fix this by adding a NULL check. > > One visible manifestation of this is that userspace programs (such as the > 'iwd' WiFi daemon) will be killed when trying to verify a TLS key using the > keyctl(2) interface. > > Cc: stable@vger.kernel.org > Fixes: 215525639631 ("X.509: support OSCCA SM2-with-SM3 certificate verification") > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> > --- > crypto/asymmetric_keys/public_key.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > index 8892908ad58c..35b09e95a870 100644 > --- a/crypto/asymmetric_keys/public_key.c > +++ b/crypto/asymmetric_keys/public_key.c > @@ -356,7 +356,7 @@ int public_key_verify_signature(const struct public_key *pkey, > if (ret) > goto error_free_key; > > - if (strcmp(sig->pkey_algo, "sm2") == 0 && sig->data_size) { > + if (sig->pkey_algo && strcmp(sig->pkey_algo, "sm2") == 0 && sig->data_size) { > ret = cert_sig_digest_update(sig, tfm); > if (ret) > goto error_free_key; >
I'm intending to use Tianjia's patch. Would you like to add a Reviewed-by?
David
---
commit 11078a592e6dcea6b9f30e822d3d30e3defc99ca
Author: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Date: Thu Jan 7 17:28:55 2021 +0800
X.509: Fix crash caused by NULL pointer
On the following call path, `sig->pkey_algo` is not assigned
in asymmetric_key_verify_signature(), which causes runtime
crash in public_key_verify_signature().
keyctl_pkey_verify
asymmetric_key_verify_signature
verify_signature
public_key_verify_signature
This patch simply check this situation and fixes the crash
caused by NULL pointer.
Fixes: 215525639631 ("X.509: support OSCCA SM2-with-SM3 certificate verification")
Cc: stable@vger.kernel.org # v5.10+
Reported-by: Tobias Markus <tobias@markus-regensburg.de>
Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Signed-off-by: David Howells <dhowells@redhat.com>
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index 8892908ad58c..788a4ba1e2e7 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -356,7 +356,8 @@ int public_key_verify_signature(const struct public_key *pkey,
if (ret)
goto error_free_key;
- if (strcmp(sig->pkey_algo, "sm2") == 0 && sig->data_size) {
+ if (sig->pkey_algo && strcmp(sig->pkey_algo, "sm2") == 0 &&
+ sig->data_size) {
ret = cert_sig_digest_update(sig, tfm);
if (ret)
goto error_free_key;
Tianjia Zhang <tianjia.zhang@linux.alibaba.com> writes: > Hi, > > I have fixed this problem last week. Still thanks for your fixing. > patch is here: https://lkml.org/lkml/2021/1/7/201 Ah, awesome! I did look if this had already been fixed, but your patch wasn't in the crypto tree and didn't think to go perusing the mailing lists. So sorry for the duplicate, and thanks for fixing this :) -Toke
David Howells <dhowells@redhat.com> writes: > I'm intending to use Tianjia's patch. Yeah, sorry for missing that. > Would you like to add a Reviewed-by? Sure: Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com> and also, if you like: Tested-by: Toke Høiland-Jørgensen <toke@redhat.com>
Toke Høiland-Jørgensen <toke@redhat.com> wrote: > Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com> > > and also, if you like: > > Tested-by: Toke Høiland-Jørgensen <toke@redhat.com> Thanks! David
On Wed, Jan 13, 2021 at 11:11:13AM +0000, David Howells wrote: > I'm intending to use Tianjia's patch. Would you like to add a Reviewed-by? > > David I can give. Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> /Jarkko > --- > commit 11078a592e6dcea6b9f30e822d3d30e3defc99ca > Author: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> > Date: Thu Jan 7 17:28:55 2021 +0800 > > X.509: Fix crash caused by NULL pointer > > On the following call path, `sig->pkey_algo` is not assigned > in asymmetric_key_verify_signature(), which causes runtime > crash in public_key_verify_signature(). > > keyctl_pkey_verify > asymmetric_key_verify_signature > verify_signature > public_key_verify_signature > > This patch simply check this situation and fixes the crash > caused by NULL pointer. > > Fixes: 215525639631 ("X.509: support OSCCA SM2-with-SM3 certificate verification") > Cc: stable@vger.kernel.org # v5.10+ > Reported-by: Tobias Markus <tobias@markus-regensburg.de> > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> > Signed-off-by: David Howells <dhowells@redhat.com> > > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > index 8892908ad58c..788a4ba1e2e7 100644 > --- a/crypto/asymmetric_keys/public_key.c > +++ b/crypto/asymmetric_keys/public_key.c > @@ -356,7 +356,8 @@ int public_key_verify_signature(const struct public_key *pkey, > if (ret) > goto error_free_key; > > - if (strcmp(sig->pkey_algo, "sm2") == 0 && sig->data_size) { > + if (sig->pkey_algo && strcmp(sig->pkey_algo, "sm2") == 0 && > + sig->data_size) { > ret = cert_sig_digest_update(sig, tfm); > if (ret) > goto error_free_key; > >
David Howells <dhowells@redhat.com> writes: > Toke Høiland-Jørgensen <toke@redhat.com> wrote: > >> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com> >> >> and also, if you like: >> >> Tested-by: Toke Høiland-Jørgensen <toke@redhat.com> > > Thanks! Any chance of that patch getting into -stable anytime soon? Would be nice to have working WiFi without having to compile my own kernels ;) -Toke
Toke Høiland-Jørgensen <toke@redhat.com> wrote: > David Howells <dhowells@redhat.com> writes: > >> Toke Høiland-Jørgensen <toke@redhat.com> wrote: >> >>> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com> >>> >>> and also, if you like: >>> >>> Tested-by: Toke Høiland-Jørgensen <toke@redhat.com> >> Thanks! > Any chance of that patch getting into -stable anytime soon? Would be > nice to have working WiFi without having to compile my own kernels ;) > > -Toke > > > I have also just tested the patch and it seems to be working with the PEAP method. I would also like to have this patch in the stable branch as I normally don't have to compile my own kernels. Also, if you want to add another tester: Tested-by: João Fonseca <jpedrofonseca@ua.pt> Thanks for the patch everyone. -João
On 19/1/21 5:09 am, João Fonseca wrote: > Toke Høiland-Jørgensen <toke@redhat.com> wrote: > >> David Howells <dhowells@redhat.com> writes: >> >>> Toke Høiland-Jørgensen <toke@redhat.com> wrote: >>> >>>> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com> >>>> >>>> and also, if you like: >>>> >>>> Tested-by: Toke Høiland-Jørgensen <toke@redhat.com> >>> Thanks! >> Any chance of that patch getting into -stable anytime soon? Would be >> nice to have working WiFi without having to compile my own kernels ;) >> >> -Toke >> >> >> > I have also just tested the patch and it seems to be working with the > PEAP method. I would also like to have this patch in the stable branch > as I normally don't have to compile my own kernels. > > Also, if you want to add another tester: > > Tested-by: João Fonseca <jpedrofonseca@ua.pt> > > Thanks for the patch everyone. > > -João > > The patch finally made it to Torvalds's tree a few hours ago, so it should hopefully land in the next stable patch. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7178a107f5ea7bdb1cc23073234f0ded0ef90ec7
On Mon, Jan 18, 2021 at 06:13:02PM +0100, Toke Høiland-Jørgensen wrote: > David Howells <dhowells@redhat.com> writes: > > > Toke Høiland-Jørgensen <toke@redhat.com> wrote: > > > >> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com> > >> > >> and also, if you like: > >> > >> Tested-by: Toke Høiland-Jørgensen <toke@redhat.com> > > > > Thanks! > > Any chance of that patch getting into -stable anytime soon? Would be > nice to have working WiFi without having to compile my own kernels ;) What ever happened to this patch? I can't seem to find it in Linus's tree anywhere :( thanks, greg k-h
Greg KH <gregkh@linuxfoundation.org> writes: > On Mon, Jan 18, 2021 at 06:13:02PM +0100, Toke Høiland-Jørgensen wrote: >> David Howells <dhowells@redhat.com> writes: >> >> > Toke Høiland-Jørgensen <toke@redhat.com> wrote: >> > >> >> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com> >> >> >> >> and also, if you like: >> >> >> >> Tested-by: Toke Høiland-Jørgensen <toke@redhat.com> >> > >> > Thanks! >> >> Any chance of that patch getting into -stable anytime soon? Would be >> nice to have working WiFi without having to compile my own kernels ;) > > What ever happened to this patch? I can't seem to find it in Linus's > tree anywhere :( This was a matter of crossed streams: Tianjia had already submitted an identical fix, which went in as: 7178a107f5ea ("X.509: Fix crash caused by NULL pointer") And that has made it into -stable, so all is well as far as I'm concerned. Sorry for the confusion! -Toke
On Mon, Mar 15, 2021 at 11:52:52AM +0100, Toke Høiland-Jørgensen wrote: > Greg KH <gregkh@linuxfoundation.org> writes: > > > On Mon, Jan 18, 2021 at 06:13:02PM +0100, Toke Høiland-Jørgensen wrote: > >> David Howells <dhowells@redhat.com> writes: > >> > >> > Toke Høiland-Jørgensen <toke@redhat.com> wrote: > >> > > >> >> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com> > >> >> > >> >> and also, if you like: > >> >> > >> >> Tested-by: Toke Høiland-Jørgensen <toke@redhat.com> > >> > > >> > Thanks! > >> > >> Any chance of that patch getting into -stable anytime soon? Would be > >> nice to have working WiFi without having to compile my own kernels ;) > > > > What ever happened to this patch? I can't seem to find it in Linus's > > tree anywhere :( > > This was a matter of crossed streams: Tianjia had already submitted an > identical fix, which went in as: > > 7178a107f5ea ("X.509: Fix crash caused by NULL pointer") > > And that has made it into -stable, so all is well as far as I'm > concerned. Sorry for the confusion! No worries, thanks for letting me know. greg k-h
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index 8892908ad58c..35b09e95a870 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -356,7 +356,7 @@ int public_key_verify_signature(const struct public_key *pkey, if (ret) goto error_free_key; - if (strcmp(sig->pkey_algo, "sm2") == 0 && sig->data_size) { + if (sig->pkey_algo && strcmp(sig->pkey_algo, "sm2") == 0 && sig->data_size) { ret = cert_sig_digest_update(sig, tfm); if (ret) goto error_free_key;
When public_key_verify_signature() is called from asymmetric_key_verify_signature(), the pkey_algo field of struct public_key_signature will be NULL, which causes a NULL pointer dereference in the strcmp() check. Fix this by adding a NULL check. One visible manifestation of this is that userspace programs (such as the 'iwd' WiFi daemon) will be killed when trying to verify a TLS key using the keyctl(2) interface. Cc: stable@vger.kernel.org Fixes: 215525639631 ("X.509: support OSCCA SM2-with-SM3 certificate verification") Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> --- crypto/asymmetric_keys/public_key.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)