| From 9a00674213a3f00394f4e3221b88f2d21fc05789 Mon Sep 17 00:00:00 2001 |
| From: Eric Biggers <ebiggers@google.com> |
| Date: Fri, 29 Dec 2017 14:30:19 -0600 |
| Subject: crypto: algapi - fix NULL dereference in crypto_remove_spawns() |
| |
| From: Eric Biggers <ebiggers@google.com> |
| |
| commit 9a00674213a3f00394f4e3221b88f2d21fc05789 upstream. |
| |
| syzkaller triggered a NULL pointer dereference in crypto_remove_spawns() |
| via a program that repeatedly and concurrently requests AEADs |
| "authenc(cmac(des3_ede-asm),pcbc-aes-aesni)" and hashes "cmac(des3_ede)" |
| through AF_ALG, where the hashes are requested as "untested" |
| (CRYPTO_ALG_TESTED is set in ->salg_mask but clear in ->salg_feat; this |
| causes the template to be instantiated for every request). |
| |
| Although AF_ALG users really shouldn't be able to request an "untested" |
| algorithm, the NULL pointer dereference is actually caused by a |
| longstanding race condition where crypto_remove_spawns() can encounter |
| an instance which has had spawn(s) "grabbed" but hasn't yet been |
| registered, resulting in ->cra_users still being NULL. |
| |
| We probably should properly initialize ->cra_users earlier, but that |
| would require updating many templates individually. For now just fix |
| the bug in a simple way that can easily be backported: make |
| crypto_remove_spawns() treat a NULL ->cra_users list as empty. |
| |
| Reported-by: syzbot <syzkaller@googlegroups.com> |
| Signed-off-by: Eric Biggers <ebiggers@google.com> |
| Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| crypto/algapi.c | 12 ++++++++++++ |
| 1 file changed, 12 insertions(+) |
| |
| --- a/crypto/algapi.c |
| +++ b/crypto/algapi.c |
| @@ -167,6 +167,18 @@ void crypto_remove_spawns(struct crypto_ |
| |
| spawn->alg = NULL; |
| spawns = &inst->alg.cra_users; |
| + |
| + /* |
| + * We may encounter an unregistered instance here, since |
| + * an instance's spawns are set up prior to the instance |
| + * being registered. An unregistered instance will have |
| + * NULL ->cra_users.next, since ->cra_users isn't |
| + * properly initialized until registration. But an |
| + * unregistered instance cannot have any users, so treat |
| + * it the same as ->cra_users being empty. |
| + */ |
| + if (spawns->next == NULL) |
| + break; |
| } |
| } while ((spawns = crypto_more_spawns(alg, &stack, &top, |
| &secondary_spawns))); |