KEYS: Fix race between read and revoke
This fixes CVE-2015-7550. 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. I think the bug was introduced with the original keyrings code. 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> Tested-by: Dmitry Vyukov <dvyukov@google.com> Cc: stable@vger.kernel.org Signed-off-by: James Morris <james.l.morris@oracle.com>
This commit is contained in:
parent
73796d8bf2
commit
b4a1b4f504
1 changed files with 9 additions and 9 deletions
|
@ -751,17 +751,17 @@ 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) */
|
||||
/* 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);
|
||||
}
|
||||
}
|
||||
|
||||
error2:
|
||||
key_put(key);
|
||||
|
|
Loading…
Reference in a new issue