CVE-2017-2647: NULL Pointer Dereference (CWE-476)
value: The KEYS subsystem in the Linux kernel before 3.18 allows local users to gain privileges or cause a denial of service (NULL pointer dereference and system crash) via vectors involving a NULL value for a certain match field, related to the keyring_search_iterator function in keyring.c.
CWE description: The product dereferences a pointer that it expects to be valid but is NULL.
CVSS v3 base score: 7.8
Published: 2017-03-31T04:59Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: crypto/asymmetric_keys/asymmetric_type.c, crypto/asymmetric_keys/pkcs7_key_type.c, fs/cifs/cifs_spnego.c, fs/cifs/cifsacl.c, fs/nfs/idmap.c, include/keys/user-type.h, include/linux/key-type.h, net/ceph/crypto.c, net/dns_resolver/dns_key.c, net/rxrpc/ar-key.c, security/keys/big_key.c, security/keys/encrypted-keys/encrypted.c, security/keys/internal.h, security/keys/key.c, security/keys/keyring.c, security/keys/request_key.c, security/keys/request_key_auth.c, security/keys/trusted.c, security/keys/user_defined.c
Commit date: 2014-09-16 17:36:06 +0100
Commit message: KEYS: Remove key_type::match in favour of overriding default by match_preparse

A previous patch added a ->match_preparse() method to the key type.  This is
allowed to override the function called by the iteration algorithm.
Therefore, we can just set a default that simply checks for an exact match of
the key description with the original criterion data and allow match_preparse
to override it as needed.

The key_type::match op is then redundant and can be removed, as can the
user_match() function.

Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>

Diff: diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index 9d78ad7754d9aa..7c04989689757d 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -57,12 +57,12 @@ EXPORT_SYMBOL_GPL(asymmetric_keyid_match);
  *
  *	"<desc>"	- request a key by description
  *	"id:<id>"	- request a key matching the ID
  *	"<subtype>:<id>" - request a key of a subtype
  */
-static int asymmetric_key_match(const struct key *key,
-				const struct key_match_data *match_data)
+static int asymmetric_key_cmp(const struct key *key,
+			      const struct key_match_data *match_data)
 {
 	const struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
 	const char *description = match_data->raw_data;
 	const char *spec = description;
 	const char *id;
@@ -108,10 +108,11 @@ static int asymmetric_key_match(const struct key *key,
  * the key is hashed according to its description.
  */
 static int asymmetric_key_match_preparse(struct key_match_data *match_data)
 {
 	match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
+	match_data->cmp = asymmetric_key_cmp;
 	return 0;
 }
 
 /*
  * Free the preparsed the match criterion.
@@ -222,11 +223,10 @@ struct key_type key_type_asymmetric = {
 	.name		= "asymmetric",
 	.preparse	= asymmetric_key_preparse,
 	.free_preparse	= asymmetric_key_free_preparse,
 	.instantiate	= generic_key_instantiate,
 	.match_preparse	= asymmetric_key_match_preparse,
-	.match		= asymmetric_key_match,
 	.match_free	= asymmetric_key_match_free,
 	.destroy	= asymmetric_key_destroy,
 	.describe	= asymmetric_key_describe,
 };
 EXPORT_SYMBOL_GPL(key_type_asymmetric);
diff --git a/crypto/asymmetric_keys/pkcs7_key_type.c b/crypto/asymmetric_keys/pkcs7_key_type.c
index d1faa1df1decdd..751f8fd7335db2 100644
--- a/crypto/asymmetric_keys/pkcs7_key_type.c
+++ b/crypto/asymmetric_keys/pkcs7_key_type.c
@@ -73,11 +73,10 @@ static int pkcs7_preparse(struct key_preparsed_payload *prep)
 static struct key_type key_type_pkcs7 = {
 	.name			= "pkcs7_test",
 	.preparse		= pkcs7_preparse,
 	.free_preparse		= user_free_preparse,
 	.instantiate		= generic_key_instantiate,
-	.match			= user_match,
 	.revoke			= user_revoke,
 	.destroy		= user_destroy,
 	.describe		= user_describe,
 	.read			= user_read,
 };
diff --git a/fs/cifs/cifs_spnego.c b/fs/cifs/cifs_spnego.c
index a3e932547617ea..f4cf200b3c7671 100644
--- a/fs/cifs/cifs_spnego.c
+++ b/fs/cifs/cifs_spnego.c
@@ -60,11 +60,10 @@ cifs_spnego_key_destroy(struct key *key)
  * keytype for CIFS spnego keys
  */
 struct key_type cifs_spnego_key_type = {
 	.name		= "cifs.spnego",
 	.instantiate	= cifs_spnego_key_instantiate,
-	.match		= user_match,
 	.destroy	= cifs_spnego_key_destroy,
 	.describe	= user_describe,
 };
 
 /* length of longest version string e.g.  strlen("ver=0xFF") */
diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
index 7ff866dbb89eb7..6d00c419cbae0b 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -82,11 +82,10 @@ cifs_idmap_key_destroy(struct key *key)
 static struct key_type cifs_idmap_key_type = {
 	.name        = "cifs.idmap",
 	.instantiate = cifs_idmap_key_instantiate,
 	.destroy     = cifs_idmap_key_destroy,
 	.describe    = user_describe,
-	.match       = user_match,
 };
 
 static char *
 sid_to_key_str(struct cifs_sid *sidptr, unsigned int type)
 {
diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index 7dd55b745c4d96..2f5db844c17253 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -175,11 +175,10 @@ static int nfs_map_numeric_to_string(__u32 id, char *buf, size_t buflen)
 static struct key_type key_type_id_resolver = {
 	.name		= "id_resolver",
 	.preparse	= user_preparse,
 	.free_preparse	= user_free_preparse,
 	.instantiate	= generic_key_instantiate,
-	.match		= user_match,
 	.revoke		= user_revoke,
 	.destroy	= user_destroy,
 	.describe	= user_describe,
 	.read		= user_read,
 };
@@ -399,11 +398,10 @@ static const struct rpc_pipe_ops idmap_upcall_ops = {
 static struct key_type key_type_id_resolver_legacy = {
 	.name		= "id_legacy",
 	.preparse	= user_preparse,
 	.free_preparse	= user_free_preparse,
 	.instantiate	= generic_key_instantiate,
-	.match		= user_match,
 	.revoke		= user_revoke,
 	.destroy	= user_destroy,
 	.describe	= user_describe,
 	.read		= user_read,
 	.request_key	= nfs_idmap_legacy_upcall,
diff --git a/include/keys/user-type.h b/include/keys/user-type.h
index 66d92af30e7c0e..cebefb069c44a5 100644
--- a/include/keys/user-type.h
+++ b/include/keys/user-type.h
@@ -34,17 +34,14 @@ struct user_key_payload {
 
 extern struct key_type key_type_user;
 extern struct key_type key_type_logon;
 
 struct key_preparsed_payload;
-struct key_match_data;
 
 extern int user_preparse(struct key_preparsed_payload *prep);
 extern void user_free_preparse(struct key_preparsed_payload *prep);
 extern int user_update(struct key *key, struct key_preparsed_payload *prep);
-extern int user_match(const struct key *key,
-		      const struct key_match_data *match_data);
 extern void user_revoke(struct key *key);
 extern void user_destroy(struct key *key);
 extern void user_describe(const struct key *user, struct seq_file *m);
 extern long user_read(const struct key *key,
 		      char __user *buffer, size_t buflen);
diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index bf93ea60927387..c14816bd3b44f1 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -111,14 +111,10 @@ struct key_type {
 	 * data to be preparsed can be found in match_data->raw_data.
 	 * The lookup type can also be set by this function.
 	 */
 	int (*match_preparse)(struct key_match_data *match_data);
 
-	/* match a key against a description */
-	int (*match)(const struct key *key,
-		     const struct key_match_data *match_data);
-
 	/* Free preparsed match data (optional).  This should be supplied it
 	 * ->match_preparse() is supplied. */
 	void (*match_free)(struct key_match_data *match_data);
 
 	/* clear some of the data from a key on revokation (optional)
diff --git a/net/ceph/crypto.c b/net/ceph/crypto.c
index ffeba8f9dda929..62fc5e7a9acf75 100644
--- a/net/ceph/crypto.c
+++ b/net/ceph/crypto.c
@@ -474,11 +474,10 @@ static void ceph_key_destroy(struct key *key)
 struct key_type key_type_ceph = {
 	.name		= "ceph",
 	.preparse	= ceph_key_preparse,
 	.free_preparse	= ceph_key_free_preparse,
 	.instantiate	= generic_key_instantiate,
-	.match		= user_match,
 	.destroy	= ceph_key_destroy,
 };
 
 int ceph_crypto_init(void) {
 	return register_key_type(&key_type_ceph);
diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index 92df6e508ae7d0..a07b9ba7e0b7bf 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -174,13 +174,12 @@ static void dns_resolver_free_preparse(struct key_preparsed_payload *prep)
  * The description is of the form "[<type>:]<domain_name>"
  *
  * The domain name may be a simple name or an absolute domain name (which
  * should end with a period).  The domain name is case-independent.
  */
-static int
-dns_resolver_match(const struct key *key,
-		   const struct key_match_data *match_data)
+static int dns_resolver_cmp(const struct key *key,
+			    const struct key_match_data *match_data)
 {
 	int slen, dlen, ret = 0;
 	const char *src = key->description, *dsp = match_data->raw_data;
 
 	kenter("%s,%s", src, dsp);
@@ -207,10 +206,20 @@ dns_resolver_match(const struct key *key,
 no_match:
 	kleave(" = %d", ret);
 	return ret;
 }
 
+/*
+ * Preparse the match criterion.
+ */
+static int dns_resolver_match_preparse(struct key_match_data *match_data)
+{
+	match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
+	match_data->cmp = dns_resolver_cmp;
+	return 0;
+}
+
 /*
  * Describe a DNS key
  */
 static void dns_resolver_describe(const struct key *key, struct seq_file *m)
 {
@@ -241,11 +250,11 @@ static long dns_resolver_read(const struct key *key,
 struct key_type key_type_dns_resolver = {
 	.name		= "dns_resolver",
 	.preparse	= dns_resolver_preparse,
 	.free_preparse	= dns_resolver_free_preparse,
 	.instantiate	= generic_key_instantiate,
-	.match		= dns_resolver_match,
+	.match_preparse	= dns_resolver_match_preparse,
 	.revoke		= user_revoke,
 	.destroy	= user_destroy,
 	.describe	= dns_resolver_describe,
 	.read		= dns_resolver_read,
 };
diff --git a/net/rxrpc/ar-key.c b/net/rxrpc/ar-key.c
index 3907add75932d9..10c6cb694b4343 100644
--- a/net/rxrpc/ar-key.c
+++ b/net/rxrpc/ar-key.c
@@ -42,11 +42,10 @@ static long rxrpc_read(const struct key *, char __user *, size_t);
 struct key_type key_type_rxrpc = {
 	.name		= "rxrpc",
 	.preparse	= rxrpc_preparse,
 	.free_preparse	= rxrpc_free_preparse,
 	.instantiate	= generic_key_instantiate,
-	.match		= user_match,
 	.destroy	= rxrpc_destroy,
 	.describe	= rxrpc_describe,
 	.read		= rxrpc_read,
 };
 EXPORT_SYMBOL(key_type_rxrpc);
@@ -59,11 +58,10 @@ struct key_type key_type_rxrpc_s = {
 	.name		= "rxrpc_s",
 	.vet_description = rxrpc_vet_description_s,
 	.preparse	= rxrpc_preparse_s,
 	.free_preparse	= rxrpc_free_preparse_s,
 	.instantiate	= generic_key_instantiate,
-	.match		= user_match,
 	.destroy	= rxrpc_destroy_s,
 	.describe	= rxrpc_describe,
 };
 
 /*
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index 4045c13a761a9f..b6adb94f6d5257 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -34,11 +34,10 @@ MODULE_LICENSE("GPL");
 struct key_type key_type_big_key = {
 	.name			= "big_key",
 	.preparse		= big_key_preparse,
 	.free_preparse		= big_key_free_preparse,
 	.instantiate		= generic_key_instantiate,
-	.match			= user_match,
 	.revoke			= big_key_revoke,
 	.destroy		= big_key_destroy,
 	.describe		= big_key_describe,
 	.read			= big_key_read,
 };
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 5fe443d120afe8..db9675db102620 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -968,11 +968,10 @@ static void encrypted_destroy(struct key *key)
 
 struct key_type key_type_encrypted = {
 	.name = "encrypted",
 	.instantiate = encrypted_instantiate,
 	.update = encrypted_update,
-	.match = user_match,
 	.destroy = encrypted_destroy,
 	.describe = user_describe,
 	.read = encrypted_read,
 };
 EXPORT_SYMBOL_GPL(key_type_encrypted);
diff --git a/security/keys/internal.h b/security/keys/internal.h
index b47cc532be1e3b..e66a16cb63e1ca 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -125,10 +125,12 @@ struct keyring_search_context {
 	bool			possessed;
 	key_ref_t		result;
 	struct timespec		now;
 };
 
+extern int key_default_cmp(const struct key *key,
+			   const struct key_match_data *match_data);
 extern key_ref_t keyring_search_aux(key_ref_t keyring_ref,
 				    struct keyring_search_context *ctx);
 
 extern key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx);
 extern key_ref_t search_process_keyrings(struct keyring_search_context *ctx);
diff --git a/security/keys/key.c b/security/keys/key.c
index b90a68c4e2c427..8c0092ca04430b 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -797,11 +797,11 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
 		key_ref = ERR_PTR(-ENODEV);
 		goto error;
 	}
 
 	key_ref = ERR_PTR(-EINVAL);
-	if (!index_key.type->match || !index_key.type->instantiate ||
+	if (!index_key.type->instantiate ||
 	    (!index_key.description && !index_key.type->preparse))
 		goto error_put_type;
 
 	keyring = key_ref_to_ptr(keyring_ref);
 
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 10f0a5f2d3620c..253c9a0eb092e2 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -87,11 +87,10 @@ struct key_type key_type_keyring = {
 	.name		= "keyring",
 	.def_datalen	= 0,
 	.preparse	= keyring_preparse,
 	.free_preparse	= keyring_free_preparse,
 	.instantiate	= keyring_instantiate,
-	.match		= user_match,
 	.revoke		= keyring_revoke,
 	.destroy	= keyring_destroy,
 	.describe	= keyring_describe,
 	.read		= keyring_read,
 };
@@ -509,10 +508,19 @@ struct key *keyring_alloc(const char *description, kuid_t uid, kgid_t gid,
 
 	return keyring;
 }
 EXPORT_SYMBOL(keyring_alloc);
 
+/*
+ * By default, we keys found by getting an exact match on their descriptions.
+ */
+int key_default_cmp(const struct key *key,
+		    const struct key_match_data *match_data)
+{
+	return strcmp(key->description, match_data->raw_data) == 0;
+}
+
 /*
  * Iteration function to consider each key found.
  */
 static int keyring_search_iterator(const void *object, void *iterator_data)
 {
@@ -882,21 +890,18 @@ key_ref_t keyring_search(key_ref_t keyring,
 {
 	struct keyring_search_context ctx = {
 		.index_key.type		= type,
 		.index_key.description	= description,
 		.cred			= current_cred(),
-		.match_data.cmp		= type->match,
+		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
 		.flags			= KEYRING_SEARCH_DO_STATE_CHECK,
 	};
 	key_ref_t key;
 	int ret;
 
-	if (!ctx.match_data.cmp)
-		return ERR_PTR(-ENOKEY);
-
 	if (type->match_preparse) {
 		ret = type->match_preparse(&ctx.match_data);
 		if (ret < 0)
 			return ERR_PTR(ret);
 	}
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 408523e5e2e2ee..dc6ed32b7844cf 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -529,11 +529,11 @@ struct key *request_key_and_link(struct key_type *type,
 {
 	struct keyring_search_context ctx = {
 		.index_key.type		= type,
 		.index_key.description	= description,
 		.cred			= current_cred(),
-		.match_data.cmp		= type->match,
+		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
 	};
 	struct key *key;
 	key_ref_t key_ref;
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 9ae02819cc064f..6639e2cb885322 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -244,11 +244,11 @@ struct key *key_get_instantiation_authkey(key_serial_t target_id)
 	char description[16];
 	struct keyring_search_context ctx = {
 		.index_key.type		= &key_type_request_key_auth,
 		.index_key.description	= description,
 		.cred			= current_cred(),
-		.match_data.cmp		= user_match,
+		.match_data.cmp		= key_default_cmp,
 		.match_data.raw_data	= description,
 		.match_data.lookup_type	= KEYRING_SEARCH_LOOKUP_DIRECT,
 	};
 	struct key *authkey;
 	key_ref_t authkey_ref;
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 6b804aa4529a17..c0594cb07adab1 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -1094,11 +1094,10 @@ static void trusted_destroy(struct key *key)
 
 struct key_type key_type_trusted = {
 	.name = "trusted",
 	.instantiate = trusted_instantiate,
 	.update = trusted_update,
-	.match = user_match,
 	.destroy = trusted_destroy,
 	.describe = user_describe,
 	.read = trusted_read,
 };
 
diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index cd7e726e8646a7..36b47bbd3d8cc2 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -28,11 +28,10 @@ struct key_type key_type_user = {
 	.name			= "user",
 	.preparse		= user_preparse,
 	.free_preparse		= user_free_preparse,
 	.instantiate		= generic_key_instantiate,
 	.update			= user_update,
-	.match			= user_match,
 	.revoke			= user_revoke,
 	.destroy		= user_destroy,
 	.describe		= user_describe,
 	.read			= user_read,
 };
@@ -49,11 +48,10 @@ struct key_type key_type_logon = {
 	.name			= "logon",
 	.preparse		= user_preparse,
 	.free_preparse		= user_free_preparse,
 	.instantiate		= generic_key_instantiate,
 	.update			= user_update,
-	.match			= user_match,
 	.revoke			= user_revoke,
 	.destroy		= user_destroy,
 	.describe		= user_describe,
 	.vet_description	= logon_vet_description,
 };
@@ -134,20 +132,10 @@ int user_update(struct key *key, struct key_preparsed_payload *prep)
 	return ret;
 }
 
 EXPORT_SYMBOL_GPL(user_update);
 
-/*
- * match users on their name
- */
-int user_match(const struct key *key, const struct key_match_data *match_data)
-{
-	return strcmp(key->description, match_data->raw_data) == 0;
-}
-
-EXPORT_SYMBOL_GPL(user_match);
-
 /*
  * dispose of the links from a revoked keyring
  * - called with the key sem write-locked
  */
 void user_revoke(struct key *key)

Vulnerable code: static int asymmetric_key_match(const struct key *key,
				const struct key_match_data *match_data)
	.match		= asymmetric_key_match,
	.match			= user_match,
	.match		= user_match,
	.match       = user_match,
	.match		= user_match,
	.match		= user_match,
struct key_match_data;
extern int user_match(const struct key *key,
		      const struct key_match_data *match_data);
	/* match a key against a description */
	int (*match)(const struct key *key,
		     const struct key_match_data *match_data);

	.match		= user_match,
static int
dns_resolver_match(const struct key *key,
		   const struct key_match_data *match_data)
	.match		= dns_resolver_match,
	.match		= user_match,
	.match		= user_match,
	.match			= user_match,
	.match = user_match,
	if (!index_key.type->match || !index_key.type->instantiate ||
	.match		= user_match,
		.match_data.cmp		= type->match,
	if (!ctx.match_data.cmp)
		return ERR_PTR(-ENOKEY);

		.match_data.cmp		= type->match,
		.match_data.cmp		= user_match,
	.match = user_match,
	.match			= user_match,
	.match			= user_match,
/*
 * match users on their name
 */
int user_match(const struct key *key, const struct key_match_data *match_data)
{
	return strcmp(key->description, match_data->raw_data) == 0;
}

EXPORT_SYMBOL_GPL(user_match);

Fixed code: static int asymmetric_key_cmp(const struct key *key,
			      const struct key_match_data *match_data)
	match_data->cmp = asymmetric_key_cmp;
static int dns_resolver_cmp(const struct key *key,
			    const struct key_match_data *match_data)
/*
 * Preparse the match criterion.
 */
static int dns_resolver_match_preparse(struct key_match_data *match_data)
{
	match_data->lookup_type = KEYRING_SEARCH_LOOKUP_ITERATE;
	match_data->cmp = dns_resolver_cmp;
	return 0;
}

	.match_preparse	= dns_resolver_match_preparse,
extern int key_default_cmp(const struct key *key,
			   const struct key_match_data *match_data);
	if (!index_key.type->instantiate ||
/*
 * By default, we keys found by getting an exact match on their descriptions.
 */
int key_default_cmp(const struct key *key,
		    const struct key_match_data *match_data)
{
	return strcmp(key->description, match_data->raw_data) == 0;
}

		.match_data.cmp		= key_default_cmp,
		.match_data.cmp		= key_default_cmp,
		.match_data.cmp		= key_default_cmp,