CVE-2016-9313: NULL Pointer Dereference (CWE-476)
value: security/keys/big_key.c in the Linux kernel before 4.8.7 mishandles unsuccessful crypto registration in conjunction with successful key-type registration, which allows local users to cause a denial of service (NULL pointer dereference and panic) or possibly have unspecified other impact via a crafted application that uses the big_key data type.
CWE description: The product dereferences a pointer that it expects to be valid but is NULL.
CVSS v3 base score: 7.8
Published: 2016-11-28T03:59Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: security/keys/big_key.c
Commit date: 2016-10-27 16:03:27 +1100
Commit message: KEYS: Sort out big_key initialisation

big_key has two separate initialisation functions, one that registers the
key type and one that registers the crypto.  If the key type fails to
register, there's no problem if the crypto registers successfully because
there's no way to reach the crypto except through the key type.

However, if the key type registers successfully but the crypto does not,
big_key_rng and big_key_blkcipher may end up set to NULL - but the code
neither checks for this nor unregisters the big key key type.

Furthermore, since the key type is registered before the crypto, it is
theoretically possible for the kernel to try adding a big_key before the
crypto is set up, leading to the same effect.

Fix this by merging big_key_crypto_init() and big_key_init() and calling
the resulting function late.  If they're going to be encrypted, we
shouldn't be creating big_keys before we have the facilities to do the
encryption available.  The key type registration is also moved after the
crypto initialisation.

The fix also includes message printing on failure.

If the big_key type isn't correctly set up, simply doing:

	dd if=/dev/zero bs=4096 count=1 | keyctl padd big_key a @s

ought to cause an oops.

Fixes: 13100a72f40f5748a04017e0ab3df4cf27c809ef ('Security: Keys: Big keys stored encrypted')
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Peter Hlavaty <zer0mem@yahoo.com>
cc: Kirill Marinushkin <k.marinushkin@gmail.com>
cc: Artem Savkov <asavkov@redhat.com>
cc: stable@vger.kernel.org
Signed-off-by: James Morris <james.l.morris@oracle.com>

Diff: diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index c0b3030b563486..835c1ab30d01eb 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -7,10 +7,11 @@
  * modify it under the terms of the GNU General Public Licence
  * as published by the Free Software Foundation; either version
  * 2 of the Licence, or (at your option) any later version.
  */
 
+#define pr_fmt(fmt) "big_key: "fmt
 #include <linux/init.h>
 #include <linux/seq_file.h>
 #include <linux/file.h>
 #include <linux/shmem_fs.h>
 #include <linux/err.h>
@@ -339,46 +340,50 @@ long big_key_read(const struct key *key, char __user *buffer, size_t buflen)
 /*
  * Register key type
  */
 static int __init big_key_init(void)
 {
-	return register_key_type(&key_type_big_key);
-}
-
-/*
- * Initialize big_key crypto and RNG algorithms
- */
-static int __init big_key_crypto_init(void)
-{
-	int ret = -EINVAL;
+	struct crypto_skcipher *cipher;
+	struct crypto_rng *rng;
+	int ret;
 
-	/* init RNG */
-	big_key_rng = crypto_alloc_rng(big_key_rng_name, 0, 0);
-	if (IS_ERR(big_key_rng)) {
-		big_key_rng = NULL;
-		return -EFAULT;
+	rng = crypto_alloc_rng(big_key_rng_name, 0, 0);
+	if (IS_ERR(rng)) {
+		pr_err("Can't alloc rng: %ld\n", PTR_ERR(rng));
+		return PTR_ERR(rng);
 	}
 
+	big_key_rng = rng;
+
 	/* seed RNG */
-	ret = crypto_rng_reset(big_key_rng, NULL, crypto_rng_seedsize(big_key_rng));
-	if (ret)
-		goto error;
+	ret = crypto_rng_reset(rng, NULL, crypto_rng_seedsize(rng));
+	if (ret) {
+		pr_err("Can't reset rng: %d\n", ret);
+		goto error_rng;
+	}
 
 	/* init block cipher */
-	big_key_skcipher = crypto_alloc_skcipher(big_key_alg_name,
-						 0, CRYPTO_ALG_ASYNC);
-	if (IS_ERR(big_key_skcipher)) {
-		big_key_skcipher = NULL;
-		ret = -EFAULT;
-		goto error;
+	cipher = crypto_alloc_skcipher(big_key_alg_name, 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(cipher)) {
+		ret = PTR_ERR(cipher);
+		pr_err("Can't alloc crypto: %d\n", ret);
+		goto error_rng;
+	}
+
+	big_key_skcipher = cipher;
+
+	ret = register_key_type(&key_type_big_key);
+	if (ret < 0) {
+		pr_err("Can't register type: %d\n", ret);
+		goto error_cipher;
 	}
 
 	return 0;
 
-error:
+error_cipher:
+	crypto_free_skcipher(big_key_skcipher);
+error_rng:
 	crypto_free_rng(big_key_rng);
-	big_key_rng = NULL;
 	return ret;
 }
 
-device_initcall(big_key_init);
-late_initcall(big_key_crypto_init);
+late_initcall(big_key_init);

Vulnerable code: 	return register_key_type(&key_type_big_key);
}

/*
 * Initialize big_key crypto and RNG algorithms
 */
static int __init big_key_crypto_init(void)
{
	int ret = -EINVAL;
	/* init RNG */
	big_key_rng = crypto_alloc_rng(big_key_rng_name, 0, 0);
	if (IS_ERR(big_key_rng)) {
		big_key_rng = NULL;
		return -EFAULT;
	ret = crypto_rng_reset(big_key_rng, NULL, crypto_rng_seedsize(big_key_rng));
	if (ret)
		goto error;
	big_key_skcipher = crypto_alloc_skcipher(big_key_alg_name,
						 0, CRYPTO_ALG_ASYNC);
	if (IS_ERR(big_key_skcipher)) {
		big_key_skcipher = NULL;
		ret = -EFAULT;
		goto error;
error:
	big_key_rng = NULL;
device_initcall(big_key_init);
late_initcall(big_key_crypto_init);
Fixed code: #define pr_fmt(fmt) "big_key: "fmt
	struct crypto_skcipher *cipher;
	struct crypto_rng *rng;
	int ret;
	rng = crypto_alloc_rng(big_key_rng_name, 0, 0);
	if (IS_ERR(rng)) {
		pr_err("Can't alloc rng: %ld\n", PTR_ERR(rng));
		return PTR_ERR(rng);
	big_key_rng = rng;

	ret = crypto_rng_reset(rng, NULL, crypto_rng_seedsize(rng));
	if (ret) {
		pr_err("Can't reset rng: %d\n", ret);
		goto error_rng;
	}
	cipher = crypto_alloc_skcipher(big_key_alg_name, 0, CRYPTO_ALG_ASYNC);
	if (IS_ERR(cipher)) {
		ret = PTR_ERR(cipher);
		pr_err("Can't alloc crypto: %d\n", ret);
		goto error_rng;
	}

	big_key_skcipher = cipher;

	ret = register_key_type(&key_type_big_key);
	if (ret < 0) {
		pr_err("Can't register type: %d\n", ret);
		goto error_cipher;
error_cipher:
	crypto_free_skcipher(big_key_skcipher);
error_rng:
late_initcall(big_key_init);