linux-kernel - [PATCH v2 2/6] KEYS: load key flags atomically in key_is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20170926201105.126166-3-ebiggers3@gmail.com>
Date:   Tue, 26 Sep 2017 13:11:01 -0700
From:   Eric Biggers <ebiggers3@...il.com>
To:     keyrings@...r.kernel.org
Cc:     David Howells <dhowells@...hat.com>,
        Michael Halcrow <mhalcrow@...gle.com>,
        linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org, Eric Biggers <ebiggers@...gle.com>
Subject: [PATCH v2 2/6] KEYS: load key flags atomically in key_is_instantiated()

From: Eric Biggers <ebiggers@...gle.com>

In key_is_instantiated(), we check for KEY_FLAG_INSTANTIATED set and
KEY_FLAG_NEGATIVE unset.  But this was done as two separate bit tests
which were not atomic with respect to each other, and had no memory
barrier providing ordering.  Therefore, it was theoretically possible
for the function to incorrectly return true if called while the key was
being negatively instantiated.

There also needs to be a memory barrier before anything which is only
meaningful for positively instantiated keys, e.g. ->payload and
->datalen, can be read --- which some of the ->describe() methods do.

Fix both these problems by loading the flags using smp_load_acquire().

Signed-off-by: Eric Biggers <ebiggers@...gle.com>
---
 include/linux/key.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index b7b590d7c480..fcb79eedbdb5 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -372,8 +372,11 @@ extern void key_set_timeout(struct key *, unsigned);
  */
 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);
+	/* Pairs with RELEASE in mark_key_instantiated() */
+	unsigned long flags = smp_load_acquire(&key->flags);
+
+	return (flags & KEY_FLAG_INSTANTIATED) &&
+		!(flags & KEY_FLAG_NEGATIVE);
 }
 
 #define dereference_key_rcu(KEY)					\
-- 
2.14.1.992.g2c7b836f3a-goog