linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers3@gmail.com>
To: linux-crypto@vger.kernel.org, Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S . Miller" <davem@davemloft.net>,
	linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
	Eric Biggers <ebiggers@google.com>,
	stable@vger.kernel.org
Subject: [PATCH] crypto: algapi - fix NULL dereference in crypto_remove_spawns()
Date: Fri, 29 Dec 2017 14:30:19 -0600	[thread overview]
Message-ID: <20171229203019.1413-1-ebiggers3@gmail.com> (raw)
In-Reply-To: <001a1141c43ad30ccf055efb76ed@google.com>

From: Eric Biggers <ebiggers@google.com>

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>
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/algapi.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/crypto/algapi.c b/crypto/algapi.c
index 9895cafcce7e..395b082d03a9 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -166,6 +166,18 @@ void crypto_remove_spawns(struct crypto_alg *alg, struct list_head *list,
 
 			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)));
-- 
2.15.1

  parent reply	other threads:[~2017-12-29 20:31 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-27 18:56 general protection fault in crypto_remove_spawns syzbot
2017-11-28 22:06 ` Stephan Müller
2017-12-12  6:09 ` [PATCH] crypto: AF_ALG - limit mask and type Stephan Müller
2017-12-12  8:57   ` Eric Biggers
2017-12-12  9:22     ` Stephan Mueller
2017-12-19  6:25   ` [PATCH v2] " Stephan Müller
2017-12-22  7:36     ` Herbert Xu
2017-12-22  7:41       ` Stephan Mueller
2017-12-22  7:58         ` Herbert Xu
2018-01-02  7:53           ` [PATCH v3] crypto: AF_ALG - whitelist " Stephan Müller
2018-01-02  7:55             ` [PATCH v4] " Stephan Müller
2018-01-12 12:23               ` Herbert Xu
2017-12-29 20:30 ` Eric Biggers [this message]
2018-01-05 11:18   ` [PATCH] crypto: algapi - fix NULL dereference in crypto_remove_spawns() Herbert Xu
2018-01-17  6:34 ` general protection fault in crypto_remove_spawns Eric Biggers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171229203019.1413-1-ebiggers3@gmail.com \
    --to=ebiggers3@gmail.com \
    --cc=davem@davemloft.net \
    --cc=ebiggers@google.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=syzkaller-bugs@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).