All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.