CVE-2015-7550 Race between read and revoke keys (rhbz 1291197 1291198)

2 files changed, 115 insertions, 0 deletions

diff --git a/KEYS-Fix-race-between-read-and-revoke.patch b/KEYS-Fix-race-between-read-and-revoke.patch
new file mode 100644
index 000000000..df0d9376b
--- /dev/null
+++ b/KEYS-Fix-race-between-read-and-revoke.patch
@@ -0,0 +1,108 @@
+From f144220f72062ed5359e0211f130670c915a12dd Mon Sep 17 00:00:00 2001
+From: David Howells <dhowells@redhat.com>
+Date: Mon, 14 Dec 2015 10:36:31 -0500
+Subject: [PATCH] KEYS: Fix race between read and revoke
+
+There's a race between keyctl_read() and keyctl_revoke().  If the revoke
+happens between keyctl_read() checking the validity of a key and the key's
+semaphore being taken, then the key type read method will see a revoked key.
+
+This causes a problem for the user-defined key type because it assumes in
+its read method that there will always be a payload in a non-revoked key
+and doesn't check for a NULL pointer.
+
+Fix this by making keyctl_read() check the validity of a key after taking
+semaphore instead of before.
+
+This was discovered by a multithreaded test program generated by syzkaller
+(http://github.com/google/syzkaller).  Here's a cleaned up version:
+
+	#include <sys/types.h>
+	#include <keyutils.h>
+	#include <pthread.h>
+	void *thr0(void *arg)
+	{
+		key_serial_t key = (unsigned long)arg;
+		keyctl_revoke(key);
+		return 0;
+	}
+	void *thr1(void *arg)
+	{
+		key_serial_t key = (unsigned long)arg;
+		char buffer[16];
+		keyctl_read(key, buffer, 16);
+		return 0;
+	}
+	int main()
+	{
+		key_serial_t key = add_key("user", "%", "foo", 3, KEY_SPEC_USER_KEYRING);
+		pthread_t th[5];
+		pthread_create(&th[0], 0, thr0, (void *)(unsigned long)key);
+		pthread_create(&th[1], 0, thr1, (void *)(unsigned long)key);
+		pthread_create(&th[2], 0, thr0, (void *)(unsigned long)key);
+		pthread_create(&th[3], 0, thr1, (void *)(unsigned long)key);
+		pthread_join(th[0], 0);
+		pthread_join(th[1], 0);
+		pthread_join(th[2], 0);
+		pthread_join(th[3], 0);
+		return 0;
+	}
+
+Build as:
+
+	cc -o keyctl-race keyctl-race.c -lkeyutils -lpthread
+
+Run as:
+
+	while keyctl-race; do :; done
+
+as it may need several iterations to crash the kernel.  The crash can be
+summarised as:
+
+	BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
+	IP: [<ffffffff81279b08>] user_read+0x56/0xa3
+	...
+	Call Trace:
+	 [<ffffffff81276aa9>] keyctl_read_key+0xb6/0xd7
+	 [<ffffffff81277815>] SyS_keyctl+0x83/0xe0
+	 [<ffffffff815dbb97>] entry_SYSCALL_64_fastpath+0x12/0x6f
+
+Reported-by: Dmitry Vyukov <dvyukov@google.com>
+Signed-off-by: David Howells <dhowells@redhat.com>
+---
+ security/keys/keyctl.c | 18 +++++++++---------
+ 1 file changed, 9 insertions(+), 9 deletions(-)
+
+diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
+index fb111eafcb89..1c3872aeed14 100644
+--- a/security/keys/keyctl.c
++++ b/security/keys/keyctl.c
+@@ -751,16 +751,16 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen)
+ 
+ 	/* the key is probably readable - now try to read it */
+ can_read_key:
+-	ret = key_validate(key);
+-	if (ret == 0) {
+-		ret = -EOPNOTSUPP;
+-		if (key->type->read) {
+-			/* read the data with the semaphore held (since we
+-			 * might sleep) */
+-			down_read(&key->sem);
++	ret = -EOPNOTSUPP;
++	if (key->type->read) {
++		/* Read the data with the semaphore held (since we might sleep)
++		 * to protect against the key being updated or revoked.
++		 */
++		down_read(&key->sem);
++		ret = key_validate(key);
++		if (ret == 0)
+ 			ret = key->type->read(key, buffer, buflen);
+-			up_read(&key->sem);
+-		}
++		up_read(&key->sem);
+ 	}
+ 
+ error2:
+-- 
+2.5.0
+
diff --git a/kernel.spec b/kernel.spec
index 58e920746..d1412371d 100644
--- a/kernel.spec
+++ b/kernel.spec
@@ -674,6 +674,9 @@ Patch573: unix-avoid-use-after-free-in-ep_remove_wait_queue.patch
 #CVE-XXXX-XXXX rhbz 1291329 1291332
 Patch574: ovl-fix-permission-checking-for-setattr.patch
 
+#CVE-2015-7550 rhbz 1291197 1291198
+Patch575: KEYS-Fix-race-between-read-and-revoke.patch
+
 # END OF PATCH DEFINITIONS
 
 %endif
@@ -1468,6 +1471,9 @@ ApplyPatch unix-avoid-use-after-free-in-ep_remove_wait_queue.patch
 #CVE-XXXX-XXXX rhbz 1291329 1291332
 ApplyPatch ovl-fix-permission-checking-for-setattr.patch
 
+#CVE-2015-7550 rhbz 1291197 1291198
+ApplyPatch KEYS-Fix-race-between-read-and-revoke.patch
+
 # END OF PATCH APPLICATIONS
 
 %endif
@@ -2319,6 +2325,7 @@ fi
 # 
 %changelog
 * Mon Dec 14 2015 Josh Boyer <jwboyer@fedoraproject.org>
+- CVE-2015-7550 Race between read and revoke keys (rhbz 1291197 1291198)
 - CVE-XXXX-XXXX permission bypass on overlayfs (rhbz 1291329 1291332)
 
 * Fri Dec 11 2015 Josh Boyer <jwboyer@fedoraproject.org>