CVE-2017-15951: Improper Input Validation (CWE-20)
value: The KEYS subsystem in the Linux kernel before 4.13.10 does not correctly synchronize the actions of updating versus finding a key in the "negative" state to avoid a race condition, which allows local users to cause a denial of service or possibly have unspecified other impact via crafted system calls.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
CVSS v3 base score: 7.8
Published: 2017-10-28T02:29Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: include/linux/key.h, net/dns_resolver/dns_key.c, security/keys/big_key.c, security/keys/encrypted-keys/encrypted.c, security/keys/gc.c, security/keys/key.c, security/keys/keyctl.c, security/keys/keyring.c, security/keys/proc.c, security/keys/process_keys.c, security/keys/request_key.c, security/keys/request_key_auth.c, security/keys/trusted.c, security/keys/user_defined.c
Commit date: 2017-10-18 09:12:40 +0100
Commit message: KEYS: Fix race between updating and finding a negative key

Consolidate KEY_FLAG_INSTANTIATED, KEY_FLAG_NEGATIVE and the rejection
error into one field such that:

 (1) The instantiation state can be modified/read atomically.

 (2) The error can be accessed atomically with the state.

 (3) The error isn't stored unioned with the payload pointers.

This deals with the problem that the state is spread over three different
objects (two bits and a separate variable) and reading or updating them
atomically isn't practical, given that not only can uninstantiated keys
change into instantiated or rejected keys, but rejected keys can also turn
into instantiated keys - and someone accessing the key might not be using
any locking.

The main side effect of this problem is that what was held in the payload
may change, depending on the state.  For instance, you might observe the
key to be in the rejected state.  You then read the cached error, but if
the key semaphore wasn't locked, the key might've become instantiated
between the two reads - and you might now have something in hand that isn't
actually an error code.

The state is now KEY_IS_UNINSTANTIATED, KEY_IS_POSITIVE or a negative error
code if the key is negatively instantiated.  The key_is_instantiated()
function is replaced with key_is_positive() to avoid confusion as negative
keys are also 'instantiated'.

Additionally, barriering is included:

 (1) Order payload-set before state-set during instantiation.

 (2) Order state-read before payload-read when using the key.

Further separate barriering is necessary if RCU is being used to access the
payload content after reading the payload pointers.

Fixes: 146aa8b1453b ("KEYS: Merge the type-specific data with the payload data")
Cc: stable@vger.kernel.org # v4.4+
Reported-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Eric Biggers <ebiggers@google.com>

Diff: diff --git a/include/linux/key.h b/include/linux/key.h
index e315e16b6ff8a0..8a15cabe928d0e 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -136,10 +136,15 @@ struct key_restriction {
 	key_restrict_link_func_t check;
 	struct key *key;
 	struct key_type *keytype;
 };
 
+enum key_state {
+	KEY_IS_UNINSTANTIATED,
+	KEY_IS_POSITIVE,		/* Positively instantiated */
+};
+
 /*****************************************************************************/
 /*
  * authentication token / access credential / keyring
  * - types of key include:
  *   - keyrings
@@ -167,29 +172,28 @@ struct key {
 	unsigned short		quotalen;	/* length added to quota */
 	unsigned short		datalen;	/* payload data length
 						 * - may not match RCU dereferenced payload
 						 * - payload should contain own length
 						 */
+	short			state;		/* Key state (+) or rejection error (-) */
 
 #ifdef KEY_DEBUGGING
 	unsigned		magic;
 #define KEY_DEBUG_MAGIC		0x18273645u
 #endif
 
 	unsigned long		flags;		/* status flags (change with bitops) */
-#define KEY_FLAG_INSTANTIATED	0	/* set if key has been instantiated */
-#define KEY_FLAG_DEAD		1	/* set if key type has been deleted */
-#define KEY_FLAG_REVOKED	2	/* set if key had been revoked */
-#define KEY_FLAG_IN_QUOTA	3	/* set if key consumes quota */
-#define KEY_FLAG_USER_CONSTRUCT	4	/* set if key is being constructed in userspace */
-#define KEY_FLAG_NEGATIVE	5	/* set if key is negative */
-#define KEY_FLAG_ROOT_CAN_CLEAR	6	/* set if key can be cleared by root without permission */
-#define KEY_FLAG_INVALIDATED	7	/* set if key has been invalidated */
-#define KEY_FLAG_BUILTIN	8	/* set if key is built in to the kernel */
-#define KEY_FLAG_ROOT_CAN_INVAL	9	/* set if key can be invalidated by root without permission */
-#define KEY_FLAG_KEEP		10	/* set if key should not be removed */
-#define KEY_FLAG_UID_KEYRING	11	/* set if key is a user or user session keyring */
+#define KEY_FLAG_DEAD		0	/* set if key type has been deleted */
+#define KEY_FLAG_REVOKED	1	/* set if key had been revoked */
+#define KEY_FLAG_IN_QUOTA	2	/* set if key consumes quota */
+#define KEY_FLAG_USER_CONSTRUCT	3	/* set if key is being constructed in userspace */
+#define KEY_FLAG_ROOT_CAN_CLEAR	4	/* set if key can be cleared by root without permission */
+#define KEY_FLAG_INVALIDATED	5	/* set if key has been invalidated */
+#define KEY_FLAG_BUILTIN	6	/* set if key is built in to the kernel */
+#define KEY_FLAG_ROOT_CAN_INVAL	7	/* set if key can be invalidated by root without permission */
+#define KEY_FLAG_KEEP		8	/* set if key should not be removed */
+#define KEY_FLAG_UID_KEYRING	9	/* set if key is a user or user session keyring */
 
 	/* the key type and key description string
 	 * - the desc is used to match a key against search criteria
 	 * - it should be a printable string
 	 * - eg: for krb5 AFS, this might be "afs@REDHAT.COM"
@@ -211,11 +215,10 @@ struct key {
 		struct {
 			/* Keyring bits */
 			struct list_head name_link;
 			struct assoc_array keys;
 		};
-		int reject_error;
 	};
 
 	/* This is set on a keyring to restrict the addition of a link to a key
 	 * to it.  If this structure isn't provided then it is assumed that the
 	 * keyring is open to any addition.  It is ignored for non-keyring
@@ -351,21 +354,31 @@ extern void key_set_timeout(struct key *, unsigned);
 #define	KEY_NEED_SEARCH	0x08	/* Require permission to search (keyring) or find (key) */
 #define	KEY_NEED_LINK	0x10	/* Require permission to link */
 #define	KEY_NEED_SETATTR 0x20	/* Require permission to change attributes */
 #define	KEY_NEED_ALL	0x3f	/* All the above permissions */
 
+static inline short key_read_state(const struct key *key)
+{
+	/* Barrier versus mark_key_instantiated(). */
+	return smp_load_acquire(&key->state);
+}
+
 /**
- * key_is_instantiated - Determine if a key has been positively instantiated
+ * key_is_positive - Determine if a key has been positively instantiated
  * @key: The key to check.
  *
  * Return true if the specified key has been positively instantiated, false
  * otherwise.
  */
-static inline bool key_is_instantiated(const struct key *key)
+static inline bool key_is_positive(const struct key *key)
+{
+	return key_read_state(key) == KEY_IS_POSITIVE;
+}
+
+static inline bool key_is_negative(const struct key *key)
 {
-	return test_bit(KEY_FLAG_INSTANTIATED, &key->flags) &&
-		!test_bit(KEY_FLAG_NEGATIVE, &key->flags);
+	return key_read_state(key) < 0;
 }
 
 #define dereference_key_rcu(KEY)					\
 	(rcu_dereference((KEY)->payload.rcu_data0))
 
diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index 8737412c7b27f3..e1d4d898a007df 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -222,11 +222,11 @@ static int dns_resolver_match_preparse(struct key_match_data *match_data)
  * Describe a DNS key
  */
 static void dns_resolver_describe(const struct key *key, struct seq_file *m)
 {
 	seq_puts(m, key->description);
-	if (key_is_instantiated(key)) {
+	if (key_is_positive(key)) {
 		int err = PTR_ERR(key->payload.data[dns_key_error]);
 
 		if (err)
 			seq_printf(m, ": %d", err);
 		else
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index e607830b6154ce..929e14978c421b 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -245,11 +245,11 @@ void big_key_revoke(struct key *key)
 {
 	struct path *path = (struct path *)&key->payload.data[big_key_path];
 
 	/* clear the quota */
 	key_payload_reserve(key, 0);
-	if (key_is_instantiated(key) &&
+	if (key_is_positive(key) &&
 	    (size_t)key->payload.data[big_key_len] > BIG_KEY_FILE_THRESHOLD)
 		vfs_truncate(path, 0);
 }
 
 /*
@@ -277,11 +277,11 @@ void big_key_describe(const struct key *key, struct seq_file *m)
 {
 	size_t datalen = (size_t)key->payload.data[big_key_len];
 
 	seq_puts(m, key->description);
 
-	if (key_is_instantiated(key))
+	if (key_is_positive(key))
 		seq_printf(m, ": %zu [%s]",
 			   datalen,
 			   datalen > BIG_KEY_FILE_THRESHOLD ? "file" : "buff");
 }
 
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 535db141f4da56..d92cbf9687c33f 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -852,11 +852,11 @@ static int encrypted_update(struct key *key, struct key_preparsed_payload *prep)
 	char *new_master_desc = NULL;
 	const char *format = NULL;
 	size_t datalen = prep->datalen;
 	int ret = 0;
 
-	if (test_bit(KEY_FLAG_NEGATIVE, &key->flags))
+	if (key_is_negative(key))
 		return -ENOKEY;
 	if (datalen <= 0 || datalen > 32767 || !prep->data)
 		return -EINVAL;
 
 	buf = kmalloc(datalen + 1, GFP_KERNEL);
diff --git a/security/keys/gc.c b/security/keys/gc.c
index 87cb260e4890f3..f01d48cb3de1aa 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -127,19 +127,19 @@ void key_gc_keytype(struct key_type *ktype)
 static noinline void key_gc_unused_keys(struct list_head *keys)
 {
 	while (!list_empty(keys)) {
 		struct key *key =
 			list_entry(keys->next, struct key, graveyard_link);
+		short state = key->state;
+
 		list_del(&key->graveyard_link);
 
 		kdebug("- %u", key->serial);
 		key_check(key);
 
 		/* Throw away the key data if the key is instantiated */
-		if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags) &&
-		    !test_bit(KEY_FLAG_NEGATIVE, &key->flags) &&
-		    key->type->destroy)
+		if (state == KEY_IS_POSITIVE && key->type->destroy)
 			key->type->destroy(key);
 
 		security_key_free(key);
 
 		/* deal with the user's key tracking and quota */
@@ -149,11 +149,11 @@ static noinline void key_gc_unused_keys(struct list_head *keys)
 			key->user->qnbytes -= key->quotalen;
 			spin_unlock(&key->user->lock);
 		}
 
 		atomic_dec(&key->user->nkeys);
-		if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags))
+		if (state != KEY_IS_UNINSTANTIATED)
 			atomic_dec(&key->user->nikeys);
 
 		key_user_put(key->user);
 
 		kfree(key->description);
diff --git a/security/keys/key.c b/security/keys/key.c
index eb914a838840df..9385e7cc710f89 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -399,10 +399,22 @@ int key_payload_reserve(struct key *key, size_t datalen)
 
 	return ret;
 }
 EXPORT_SYMBOL(key_payload_reserve);
 
+/*
+ * Change the key state to being instantiated.
+ */
+static void mark_key_instantiated(struct key *key, int reject_error)
+{
+	/* Commit the payload before setting the state; barrier versus
+	 * key_read_state().
+	 */
+	smp_store_release(&key->state,
+			  (reject_error < 0) ? reject_error : KEY_IS_POSITIVE);
+}
+
 /*
  * Instantiate a key and link it into the target keyring atomically.  Must be
  * called with the target keyring's semaphore writelocked.  The target key's
  * semaphore need not be locked as instantiation is serialised by
  * key_construction_mutex.
@@ -422,18 +434,18 @@ static int __key_instantiate_and_link(struct key *key,
 	ret = -EBUSY;
 
 	mutex_lock(&key_construction_mutex);
 
 	/* can't instantiate twice */
-	if (!test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) {
+	if (key->state == KEY_IS_UNINSTANTIATED) {
 		/* instantiate the key */
 		ret = key->type->instantiate(key, prep);
 
 		if (ret == 0) {
 			/* mark the key as being instantiated */
 			atomic_inc(&key->user->nikeys);
-			set_bit(KEY_FLAG_INSTANTIATED, &key->flags);
+			mark_key_instantiated(key, 0);
 
 			if (test_and_clear_bit(KEY_FLAG_USER_CONSTRUCT, &key->flags))
 				awaken = 1;
 
 			/* and link it into the destination keyring */
@@ -575,17 +587,14 @@ int key_reject_and_link(struct key *key,
 	}
 
 	mutex_lock(&key_construction_mutex);
 
 	/* can't instantiate twice */
-	if (!test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) {
+	if (key->state == KEY_IS_UNINSTANTIATED) {
 		/* mark the key as being negatively instantiated */
 		atomic_inc(&key->user->nikeys);
-		key->reject_error = -error;
-		smp_wmb();
-		set_bit(KEY_FLAG_NEGATIVE, &key->flags);
-		set_bit(KEY_FLAG_INSTANTIATED, &key->flags);
+		mark_key_instantiated(key, -error);
 		now = current_kernel_time();
 		key->expiry = now.tv_sec + timeout;
 		key_schedule_gc(key->expiry + key_gc_delay);
 
 		if (test_and_clear_bit(KEY_FLAG_USER_CONSTRUCT, &key->flags))
@@ -750,12 +759,12 @@ static inline key_ref_t __key_update(key_ref_t key_ref,
 
 	down_write(&key->sem);
 
 	ret = key->type->update(key, prep);
 	if (ret == 0)
-		/* updating a negative key instantiates it */
-		clear_bit(KEY_FLAG_NEGATIVE, &key->flags);
+		/* Updating a negative key positively instantiates it */
+		mark_key_instantiated(key, 0);
 
 	up_write(&key->sem);
 
 	if (ret < 0)
 		goto error;
@@ -984,12 +993,12 @@ int key_update(key_ref_t key_ref, const void *payload, size_t plen)
 
 	down_write(&key->sem);
 
 	ret = key->type->update(key, &prep);
 	if (ret == 0)
-		/* updating a negative key instantiates it */
-		clear_bit(KEY_FLAG_NEGATIVE, &key->flags);
+		/* Updating a negative key positively instantiates it */
+		mark_key_instantiated(key, 0);
 
 	up_write(&key->sem);
 
 error:
 	if (key->type->preparse)
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 365ff85d7e2712..76d22f726ae49d 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -764,14 +764,13 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen)
 		goto error;
 	}
 
 	key = key_ref_to_ptr(key_ref);
 
-	if (test_bit(KEY_FLAG_NEGATIVE, &key->flags)) {
-		ret = -ENOKEY;
-		goto error2;
-	}
+	ret = key_read_state(key);
+	if (ret < 0)
+		goto error2; /* Negatively instantiated */
 
 	/* see if we can read it directly */
 	ret = key_permission(key_ref, KEY_NEED_READ);
 	if (ret == 0)
 		goto can_read_key;
@@ -899,11 +898,11 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group)
 		}
 
 		atomic_dec(&key->user->nkeys);
 		atomic_inc(&newowner->nkeys);
 
-		if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) {
+		if (key->state != KEY_IS_UNINSTANTIATED) {
 			atomic_dec(&key->user->nikeys);
 			atomic_inc(&newowner->nikeys);
 		}
 
 		zapowner = key->user;
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 4fa82a8a9c0e6c..06173b091a7481 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -412,11 +412,11 @@ static void keyring_describe(const struct key *keyring, struct seq_file *m)
 	if (keyring->description)
 		seq_puts(m, keyring->description);
 	else
 		seq_puts(m, "[anon]");
 
-	if (key_is_instantiated(keyring)) {
+	if (key_is_positive(keyring)) {
 		if (keyring->keys.nr_leaves_on_tree != 0)
 			seq_printf(m, ": %lu", keyring->keys.nr_leaves_on_tree);
 		else
 			seq_puts(m, ": empty");
 	}
@@ -551,11 +551,12 @@ bool key_default_cmp(const struct key *key,
  */
 static int keyring_search_iterator(const void *object, void *iterator_data)
 {
 	struct keyring_search_context *ctx = iterator_data;
 	const struct key *key = keyring_ptr_to_key(object);
-	unsigned long kflags = key->flags;
+	unsigned long kflags = READ_ONCE(key->flags);
+	short state = READ_ONCE(key->state);
 
 	kenter("{%d}", key->serial);
 
 	/* ignore keys not of this type */
 	if (key->type != ctx->index_key.type) {
@@ -595,13 +596,12 @@ static int keyring_search_iterator(const void *object, void *iterator_data)
 		goto skipped;
 	}
 
 	if (ctx->flags & KEYRING_SEARCH_DO_STATE_CHECK) {
 		/* we set a different error code if we pass a negative key */
-		if (kflags & (1 << KEY_FLAG_NEGATIVE)) {
-			smp_rmb();
-			ctx->result = ERR_PTR(key->reject_error);
+		if (state < 0) {
+			ctx->result = ERR_PTR(state);
 			kleave(" = %d [neg]", ctx->skipped_ret);
 			goto skipped;
 		}
 	}
 
diff --git a/security/keys/proc.c b/security/keys/proc.c
index de834309d10020..4089ce1f7757d3 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -180,10 +180,11 @@ static int proc_keys_show(struct seq_file *m, void *v)
 	struct key *key = rb_entry(_p, struct key, serial_node);
 	struct timespec now;
 	unsigned long timo;
 	key_ref_t key_ref, skey_ref;
 	char xbuf[16];
+	short state;
 	int rc;
 
 	struct keyring_search_context ctx = {
 		.index_key.type		= key->type,
 		.index_key.description	= key->description,
@@ -234,21 +235,23 @@ static int proc_keys_show(struct seq_file *m, void *v)
 			sprintf(xbuf, "%lud", timo / (60*60*24));
 		else
 			sprintf(xbuf, "%luw", timo / (60*60*24*7));
 	}
 
+	state = key_read_state(key);
+
 #define showflag(KEY, LETTER, FLAG) \
 	(test_bit(FLAG,	&(KEY)->flags) ? LETTER : '-')
 
 	seq_printf(m, "%08x %c%c%c%c%c%c%c %5d %4s %08x %5d %5d %-9.9s ",
 		   key->serial,
-		   showflag(key, 'I', KEY_FLAG_INSTANTIATED),
+		   state != KEY_IS_UNINSTANTIATED ? 'I' : '-',
 		   showflag(key, 'R', KEY_FLAG_REVOKED),
 		   showflag(key, 'D', KEY_FLAG_DEAD),
 		   showflag(key, 'Q', KEY_FLAG_IN_QUOTA),
 		   showflag(key, 'U', KEY_FLAG_USER_CONSTRUCT),
-		   showflag(key, 'N', KEY_FLAG_NEGATIVE),
+		   state < 0 ? 'N' : '-',
 		   showflag(key, 'i', KEY_FLAG_INVALIDATED),
 		   refcount_read(&key->usage),
 		   xbuf,
 		   key->perm,
 		   from_kuid_munged(seq_user_ns(m), key->uid),
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 293d3598153bf0..740affd65ee984 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -728,11 +728,11 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 			goto invalid_key;
 	}
 
 	ret = -EIO;
 	if (!(lflags & KEY_LOOKUP_PARTIAL) &&
-	    !test_bit(KEY_FLAG_INSTANTIATED, &key->flags))
+	    key_read_state(key) == KEY_IS_UNINSTANTIATED)
 		goto invalid_key;
 
 	/* check the permissions */
 	ret = key_task_permission(key_ref, ctx.cred, perm);
 	if (ret < 0)
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 63e63a42db3c0a..e8036cd0ad5430 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -593,14 +593,13 @@ int wait_for_key_construction(struct key *key, bool intr)
 
 	ret = wait_on_bit(&key->flags, KEY_FLAG_USER_CONSTRUCT,
 			  intr ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE);
 	if (ret)
 		return -ERESTARTSYS;
-	if (test_bit(KEY_FLAG_NEGATIVE, &key->flags)) {
-		smp_rmb();
-		return key->reject_error;
-	}
+	ret = key_read_state(key);
+	if (ret < 0)
+		return ret;
 	return key_validate(key);
 }
 EXPORT_SYMBOL(wait_for_key_construction);
 
 /**
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index 6ebf1af8fce963..424e1d90412ea0 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -71,11 +71,11 @@ static void request_key_auth_describe(const struct key *key,
 {
 	struct request_key_auth *rka = key->payload.data[0];
 
 	seq_puts(m, "key:");
 	seq_puts(m, key->description);
-	if (key_is_instantiated(key))
+	if (key_is_positive(key))
 		seq_printf(m, " pid:%d ci:%zu", rka->pid, rka->callout_len);
 }
 
 /*
  * Read the callout_info data (retrieves the callout information).
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index ddfaebf60fc864..bd85315cbfeb3e 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -1064,11 +1064,11 @@ static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
 	struct trusted_key_options *new_o;
 	size_t datalen = prep->datalen;
 	char *datablob;
 	int ret = 0;
 
-	if (test_bit(KEY_FLAG_NEGATIVE, &key->flags))
+	if (key_is_negative(key))
 		return -ENOKEY;
 	p = key->payload.data[0];
 	if (!p->migratable)
 		return -EPERM;
 	if (datalen <= 0 || datalen > 32767 || !prep->data)
diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index 3d8c68eba51602..9f558bedba23a3 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -112,11 +112,11 @@ int user_update(struct key *key, struct key_preparsed_payload *prep)
 	if (ret < 0)
 		return ret;
 
 	/* attach the new data, displacing the old */
 	key->expiry = prep->expiry;
-	if (!test_bit(KEY_FLAG_NEGATIVE, &key->flags))
+	if (key_is_positive(key))
 		zap = dereference_key_locked(key);
 	rcu_assign_keypointer(key, prep->payload.data[0]);
 	prep->payload.data[0] = NULL;
 
 	if (zap)
@@ -160,11 +160,11 @@ EXPORT_SYMBOL_GPL(user_destroy);
  * describe the user key
  */
 void user_describe(const struct key *key, struct seq_file *m)
 {
 	seq_puts(m, key->description);
-	if (key_is_instantiated(key))
+	if (key_is_positive(key))
 		seq_printf(m, ": %u", key->datalen);
 }
 
 EXPORT_SYMBOL_GPL(user_describe);
 

Vulnerable code: #define KEY_FLAG_INSTANTIATED	0	/* set if key has been instantiated */
#define KEY_FLAG_DEAD		1	/* set if key type has been deleted */
#define KEY_FLAG_REVOKED	2	/* set if key had been revoked */
#define KEY_FLAG_IN_QUOTA	3	/* set if key consumes quota */
#define KEY_FLAG_USER_CONSTRUCT	4	/* set if key is being constructed in userspace */
#define KEY_FLAG_NEGATIVE	5	/* set if key is negative */
#define KEY_FLAG_ROOT_CAN_CLEAR	6	/* set if key can be cleared by root without permission */
#define KEY_FLAG_INVALIDATED	7	/* set if key has been invalidated */
#define KEY_FLAG_BUILTIN	8	/* set if key is built in to the kernel */
#define KEY_FLAG_ROOT_CAN_INVAL	9	/* set if key can be invalidated by root without permission */
#define KEY_FLAG_KEEP		10	/* set if key should not be removed */
#define KEY_FLAG_UID_KEYRING	11	/* set if key is a user or user session keyring */
		int reject_error;
 * key_is_instantiated - Determine if a key has been positively instantiated
static inline bool key_is_instantiated(const struct key *key)
	return test_bit(KEY_FLAG_INSTANTIATED, &key->flags) &&
		!test_bit(KEY_FLAG_NEGATIVE, &key->flags);
	if (key_is_instantiated(key)) {
	if (key_is_instantiated(key) &&
	if (key_is_instantiated(key))
	if (test_bit(KEY_FLAG_NEGATIVE, &key->flags))
		if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags) &&
		    !test_bit(KEY_FLAG_NEGATIVE, &key->flags) &&
		    key->type->destroy)
		if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags))
	if (!test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) {
			set_bit(KEY_FLAG_INSTANTIATED, &key->flags);
	if (!test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) {
		key->reject_error = -error;
		smp_wmb();
		set_bit(KEY_FLAG_NEGATIVE, &key->flags);
		set_bit(KEY_FLAG_INSTANTIATED, &key->flags);
		/* updating a negative key instantiates it */
		clear_bit(KEY_FLAG_NEGATIVE, &key->flags);
		/* updating a negative key instantiates it */
		clear_bit(KEY_FLAG_NEGATIVE, &key->flags);
	if (test_bit(KEY_FLAG_NEGATIVE, &key->flags)) {
		ret = -ENOKEY;
		goto error2;
	}
		if (test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) {
	if (key_is_instantiated(keyring)) {
	unsigned long kflags = key->flags;
		if (kflags & (1 << KEY_FLAG_NEGATIVE)) {
			smp_rmb();
			ctx->result = ERR_PTR(key->reject_error);
		   showflag(key, 'I', KEY_FLAG_INSTANTIATED),
		   showflag(key, 'N', KEY_FLAG_NEGATIVE),
	    !test_bit(KEY_FLAG_INSTANTIATED, &key->flags))
	if (test_bit(KEY_FLAG_NEGATIVE, &key->flags)) {
		smp_rmb();
		return key->reject_error;
	}
	if (key_is_instantiated(key))
	if (test_bit(KEY_FLAG_NEGATIVE, &key->flags))
	if (!test_bit(KEY_FLAG_NEGATIVE, &key->flags))
	if (key_is_instantiated(key))
Fixed code: enum key_state {
	KEY_IS_UNINSTANTIATED,
	KEY_IS_POSITIVE,		/* Positively instantiated */
};

	short			state;		/* Key state (+) or rejection error (-) */
#define KEY_FLAG_DEAD		0	/* set if key type has been deleted */
#define KEY_FLAG_REVOKED	1	/* set if key had been revoked */
#define KEY_FLAG_IN_QUOTA	2	/* set if key consumes quota */
#define KEY_FLAG_USER_CONSTRUCT	3	/* set if key is being constructed in userspace */
#define KEY_FLAG_ROOT_CAN_CLEAR	4	/* set if key can be cleared by root without permission */
#define KEY_FLAG_INVALIDATED	5	/* set if key has been invalidated */
#define KEY_FLAG_BUILTIN	6	/* set if key is built in to the kernel */
#define KEY_FLAG_ROOT_CAN_INVAL	7	/* set if key can be invalidated by root without permission */
#define KEY_FLAG_KEEP		8	/* set if key should not be removed */
#define KEY_FLAG_UID_KEYRING	9	/* set if key is a user or user session keyring */
static inline short key_read_state(const struct key *key)
{
	/* Barrier versus mark_key_instantiated(). */
	return smp_load_acquire(&key->state);
}

 * key_is_positive - Determine if a key has been positively instantiated
static inline bool key_is_positive(const struct key *key)
{
	return key_read_state(key) == KEY_IS_POSITIVE;
}

static inline bool key_is_negative(const struct key *key)
	return key_read_state(key) < 0;
	if (key_is_positive(key)) {
	if (key_is_positive(key) &&
	if (key_is_positive(key))
	if (key_is_negative(key))
		short state = key->state;

		if (state == KEY_IS_POSITIVE && key->type->destroy)
		if (state != KEY_IS_UNINSTANTIATED)
/*
 * Change the key state to being instantiated.
 */
static void mark_key_instantiated(struct key *key, int reject_error)
{
	/* Commit the payload before setting the state; barrier versus
	 * key_read_state().
	 */
	smp_store_release(&key->state,
			  (reject_error < 0) ? reject_error : KEY_IS_POSITIVE);
}

	if (key->state == KEY_IS_UNINSTANTIATED) {
			mark_key_instantiated(key, 0);
	if (key->state == KEY_IS_UNINSTANTIATED) {
		mark_key_instantiated(key, -error);
		/* Updating a negative key positively instantiates it */
		mark_key_instantiated(key, 0);
		/* Updating a negative key positively instantiates it */
		mark_key_instantiated(key, 0);
	ret = key_read_state(key);
	if (ret < 0)
		goto error2; /* Negatively instantiated */
		if (key->state != KEY_IS_UNINSTANTIATED) {
	if (key_is_positive(keyring)) {
	unsigned long kflags = READ_ONCE(key->flags);
	short state = READ_ONCE(key->state);
		if (state < 0) {
			ctx->result = ERR_PTR(state);
	short state;
	state = key_read_state(key);

		   state != KEY_IS_UNINSTANTIATED ? 'I' : '-',
		   state < 0 ? 'N' : '-',
	    key_read_state(key) == KEY_IS_UNINSTANTIATED)
	ret = key_read_state(key);
	if (ret < 0)
		return ret;
	if (key_is_positive(key))
	if (key_is_negative(key))
	if (key_is_positive(key))
	if (key_is_positive(key))