Linux v4.14-rc5-94-g9a27ded2195a
This commit is contained in:
Justin M. Forbes
parent
59566d9a2c
commit
135abd0c28
@ -1,258 +0,0 @@
|
||||
From 4b244721c11c2f66052ceadd8ef6c48a53290e10 Mon Sep 17 00:00:00 2001
|
||||
From: Eric Biggers <ebiggers@google.com>
|
||||
Date: Wed, 27 Sep 2017 12:50:42 -0700
|
||||
Subject: [PATCH] KEYS: fix race between updating and finding negative key
|
||||
|
||||
In keyring_search_iterator() and in wait_for_key_construction(), we
|
||||
check whether the key has been negatively instantiated, and if so return
|
||||
the key's ->reject_error.
|
||||
|
||||
However, no lock is held during this, and ->reject_error is in union
|
||||
with ->payload. And it's impossible for KEY_FLAG_NEGATIVE to be updated
|
||||
atomically with respect to ->reject_error and ->payload.
|
||||
|
||||
Most problematically, when a negative key is positively instantiated via
|
||||
__key_update() (via sys_add_key()), ->payload is initialized first, then
|
||||
KEY_FLAG_NEGATIVE is cleared. But that means that ->reject_error can be
|
||||
observed to have a bogus value, having been overwritten with ->payload,
|
||||
while the key still appears to be "negative". Clearing
|
||||
KEY_FLAG_NEGATIVE first wouldn't work either, since then anyone who
|
||||
accesses the payload under rcu_read_lock() rather than the key semaphore
|
||||
might observe an uninitialized ->payload. Nor can we just always take
|
||||
the key's semaphore when checking whether the key is negative, since
|
||||
keyring searches happen under rcu_read_lock().
|
||||
|
||||
Therefore, fix the bug by moving ->reject_error into the high bits of
|
||||
->flags so that we can read and write it atomically with respect to
|
||||
KEY_FLAG_NEGATIVE and KEY_FLAG_INSTANTIATED.
|
||||
|
||||
This will also allow KEY_FLAG_NEGATIVE to be removed, since tests for
|
||||
KEY_FLAG_NEGATIVE can be replaced with tests for nonzero reject_error.
|
||||
But for ease of backporting this fix, that is left for a later patch.
|
||||
|
||||
This fixes a kernel crash caused by the following program:
|
||||
|
||||
#include <stdlib.h>
|
||||
#include <unistd.h>
|
||||
#include <keyutils.h>
|
||||
|
||||
int main(void)
|
||||
{
|
||||
int ringid = keyctl_join_session_keyring(NULL);
|
||||
|
||||
if (fork()) {
|
||||
for (;;) {
|
||||
usleep(rand() % 4096);
|
||||
add_key("user", "desc", "x", 1, ringid);
|
||||
keyctl_clear(ringid);
|
||||
}
|
||||
} else {
|
||||
for (;;)
|
||||
request_key("user", "desc", "", ringid);
|
||||
}
|
||||
}
|
||||
|
||||
Here is the crash:
|
||||
|
||||
BUG: unable to handle kernel paging request at fffffffffd39a6b0
|
||||
IP: __key_link_begin+0x0/0x100
|
||||
PGD 7a0a067 P4D 7a0a067 PUD 7a0c067 PMD 0
|
||||
Oops: 0000 [#1] SMP
|
||||
CPU: 1 PID: 165 Comm: keyctl_negate_r Not tainted 4.14.0-rc1 #377
|
||||
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-20170228_101828-anatol 04/01/2014
|
||||
task: ffff9791fd809140 task.stack: ffffacba402bc000
|
||||
RIP: 0010:__key_link_begin+0x0/0x100
|
||||
RSP: 0018:ffffacba402bfdc8 EFLAGS: 00010282
|
||||
RAX: ffff9791fd809140 RBX: fffffffffd39a620 RCX: 0000000000000008
|
||||
RDX: ffffacba402bfdd0 RSI: fffffffffd39a6a0 RDI: ffff9791fd810600
|
||||
RBP: ffffacba402bfdf8 R08: 0000000000000063 R09: ffffffff94845620
|
||||
R10: 8080808080808080 R11: 0000000000000004 R12: ffff9791fd810600
|
||||
R13: ffff9791fd39a940 R14: fffffffffd39a6a0 R15: 0000000000000000
|
||||
FS: 00007fbf14a90740(0000) GS:ffff9791ffd00000(0000) knlGS:0000000000000000
|
||||
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
|
||||
CR2: fffffffffd39a6b0 CR3: 000000003b910003 CR4: 00000000003606e0
|
||||
Call Trace:
|
||||
? key_link+0x28/0xb0
|
||||
? search_process_keyrings+0x13/0x100
|
||||
request_key_and_link+0xcb/0x550
|
||||
? keyring_instantiate+0x110/0x110
|
||||
? key_default_cmp+0x20/0x20
|
||||
SyS_request_key+0xc0/0x160
|
||||
? exit_to_usermode_loop+0x5e/0x80
|
||||
entry_SYSCALL_64_fastpath+0x1a/0xa5
|
||||
RIP: 0033:0x7fbf14190bb9
|
||||
RSP: 002b:00007ffd8e4fe6c8 EFLAGS: 00000246 ORIG_RAX: 00000000000000f9
|
||||
RAX: ffffffffffffffda RBX: 0000000036cc28fb RCX: 00007fbf14190bb9
|
||||
RDX: 000055748b56ca4a RSI: 000055748b56ca46 RDI: 000055748b56ca4b
|
||||
RBP: 000055748b56ca4a R08: 0000000000000001 R09: 0000000000000001
|
||||
R10: 0000000036cc28fb R11: 0000000000000246 R12: 000055748b56c8b0
|
||||
R13: 00007ffd8e4fe7d0 R14: 0000000000000000 R15: 0000000000000000
|
||||
Code: c5 0f 85 69 ff ff ff 48 c7 c3 82 ff ff ff eb ab 45 31 ed e9 18 ff ff ff 85 c0 75 8d eb d2 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 <48> 83 7e 10 00 0f 84 c5 00 00 00 55 48 89 e5 41 57 41 56 41 55
|
||||
RIP: __key_link_begin+0x0/0x100 RSP: ffffacba402bfdc8
|
||||
CR2: fffffffffd39a6b0
|
||||
|
||||
Fixes: 146aa8b1453b ("KEYS: Merge the type-specific data with the payload data")
|
||||
Cc: <stable@vger.kernel.org> [v4.4+]
|
||||
Signed-off-by: Eric Biggers <ebiggers@google.com>
|
||||
---
|
||||
include/linux/key.h | 12 +++++++++++-
|
||||
security/keys/key.c | 26 +++++++++++++++++++-------
|
||||
security/keys/keyctl.c | 3 +++
|
||||
security/keys/keyring.c | 4 ++--
|
||||
security/keys/request_key.c | 11 +++++++----
|
||||
5 files changed, 42 insertions(+), 14 deletions(-)
|
||||
|
||||
diff --git a/include/linux/key.h b/include/linux/key.h
|
||||
index e315e16b6ff8..b7b590d7c480 100644
|
||||
--- a/include/linux/key.h
|
||||
+++ b/include/linux/key.h
|
||||
@@ -189,6 +189,17 @@ struct key {
|
||||
#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 */
|
||||
|
||||
+ /*
|
||||
+ * If the key is negatively instantiated, then bits 20-31 hold the error
|
||||
+ * code which should be returned when someone tries to use the key
|
||||
+ * (unless they allow negative keys). The error code is stored as a
|
||||
+ * positive number, so it must be negated before being returned.
|
||||
+ *
|
||||
+ * Note that a key can go from negative to positive but not vice versa.
|
||||
+ */
|
||||
+#define KEY_FLAGS_REJECT_ERROR_SHIFT 20
|
||||
+#define KEY_FLAGS_REJECT_ERROR_MASK 0xFFF00000
|
||||
+
|
||||
/* the key type and key description string
|
||||
* - the desc is used to match a key against search criteria
|
||||
* - it should be a printable string
|
||||
@@ -213,7 +224,6 @@ struct key {
|
||||
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
|
||||
diff --git a/security/keys/key.c b/security/keys/key.c
|
||||
index eb914a838840..786158d3442e 100644
|
||||
--- a/security/keys/key.c
|
||||
+++ b/security/keys/key.c
|
||||
@@ -401,6 +401,20 @@ int key_payload_reserve(struct key *key, size_t datalen)
|
||||
}
|
||||
EXPORT_SYMBOL(key_payload_reserve);
|
||||
|
||||
+static void mark_key_instantiated(struct key *key, unsigned int reject_error)
|
||||
+{
|
||||
+ unsigned long old, new;
|
||||
+
|
||||
+ do {
|
||||
+ old = READ_ONCE(key->flags);
|
||||
+ new = (old & ~((1 << KEY_FLAG_NEGATIVE) |
|
||||
+ KEY_FLAGS_REJECT_ERROR_MASK)) |
|
||||
+ (1 << KEY_FLAG_INSTANTIATED) |
|
||||
+ (reject_error ? (1 << KEY_FLAG_NEGATIVE) : 0) |
|
||||
+ (reject_error << KEY_FLAGS_REJECT_ERROR_SHIFT);
|
||||
+ } while (cmpxchg_release(&key->flags, old, new) != old);
|
||||
+}
|
||||
+
|
||||
/*
|
||||
* 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
|
||||
@@ -431,7 +445,7 @@ static int __key_instantiate_and_link(struct key *key,
|
||||
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;
|
||||
@@ -580,10 +594,8 @@ int key_reject_and_link(struct key *key,
|
||||
if (!test_bit(KEY_FLAG_INSTANTIATED, &key->flags)) {
|
||||
/* 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);
|
||||
@@ -753,7 +765,7 @@ static inline key_ref_t __key_update(key_ref_t key_ref,
|
||||
ret = key->type->update(key, prep);
|
||||
if (ret == 0)
|
||||
/* updating a negative key instantiates it */
|
||||
- clear_bit(KEY_FLAG_NEGATIVE, &key->flags);
|
||||
+ mark_key_instantiated(key, 0);
|
||||
|
||||
up_write(&key->sem);
|
||||
|
||||
@@ -987,7 +999,7 @@ int key_update(key_ref_t key_ref, const void *payload, size_t plen)
|
||||
ret = key->type->update(key, &prep);
|
||||
if (ret == 0)
|
||||
/* updating a negative key instantiates it */
|
||||
- clear_bit(KEY_FLAG_NEGATIVE, &key->flags);
|
||||
+ mark_key_instantiated(key, 0);
|
||||
|
||||
up_write(&key->sem);
|
||||
|
||||
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
|
||||
index 365ff85d7e27..19a09e121089 100644
|
||||
--- a/security/keys/keyctl.c
|
||||
+++ b/security/keys/keyctl.c
|
||||
@@ -1223,6 +1223,9 @@ long keyctl_reject_key(key_serial_t id, unsigned timeout, unsigned error,
|
||||
error == ERESTART_RESTARTBLOCK)
|
||||
return -EINVAL;
|
||||
|
||||
+ BUILD_BUG_ON(MAX_ERRNO > (KEY_FLAGS_REJECT_ERROR_MASK >>
|
||||
+ KEY_FLAGS_REJECT_ERROR_SHIFT));
|
||||
+
|
||||
/* the appropriate instantiation authorisation key must have been
|
||||
* assumed before calling this */
|
||||
ret = -EPERM;
|
||||
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
|
||||
index 129a4175760b..e54ad0ed7aa4 100644
|
||||
--- a/security/keys/keyring.c
|
||||
+++ b/security/keys/keyring.c
|
||||
@@ -598,8 +598,8 @@ static int keyring_search_iterator(const void *object, void *iterator_data)
|
||||
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);
|
||||
+ ctx->result = ERR_PTR(-(int)(kflags >>
|
||||
+ KEY_FLAGS_REJECT_ERROR_SHIFT));
|
||||
kleave(" = %d [neg]", ctx->skipped_ret);
|
||||
goto skipped;
|
||||
}
|
||||
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
|
||||
index 63e63a42db3c..0aab68344837 100644
|
||||
--- a/security/keys/request_key.c
|
||||
+++ b/security/keys/request_key.c
|
||||
@@ -590,15 +590,18 @@ struct key *request_key_and_link(struct key_type *type,
|
||||
int wait_for_key_construction(struct key *key, bool intr)
|
||||
{
|
||||
int ret;
|
||||
+ unsigned long flags;
|
||||
|
||||
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;
|
||||
- }
|
||||
+
|
||||
+ /* Pairs with RELEASE in mark_key_instantiated() */
|
||||
+ flags = smp_load_acquire(&key->flags);
|
||||
+ if (flags & (1 << KEY_FLAG_NEGATIVE))
|
||||
+ return -(int)(flags >> KEY_FLAGS_REJECT_ERROR_SHIFT);
|
||||
+
|
||||
return key_validate(key);
|
||||
}
|
||||
EXPORT_SYMBOL(wait_for_key_construction);
|
||||
--
|
||||
2.13.6
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
2
gitrev
2
gitrev
@ -1 +1 @@
|
||||
73d3393ada4f70fa3df5639c8d438f2f034c0ecb
|
||||
9a27ded2195aaec2041ed2525ba7f373c60920c7
|
||||
|
||||
@ -69,7 +69,7 @@ Summary: The Linux kernel
|
||||
# The rc snapshot level
|
||||
%global rcrev 5
|
||||
# The git snapshot level
|
||||
%define gitrev 3
|
||||
%define gitrev 4
|
||||
# Set rpm version accordingly
|
||||
%define rpmversion 4.%{upstream_sublevel}.0
|
||||
%endif
|
||||
@ -639,7 +639,6 @@ Patch502: CVE-2017-7477.patch
|
||||
|
||||
# rhbz 1498016 1498017
|
||||
Patch503: KEYS-don-t-let-add_key-update-an-uninstantiated-key.patch
|
||||
Patch504: KEYS-fix-race-between-updating-and-finding-negative-.patch
|
||||
|
||||
# 600 - Patches for improved Bay and Cherry Trail device support
|
||||
# Below patches are submitted upstream, awaiting review / merging
|
||||
@ -2213,6 +2212,9 @@ fi
|
||||
#
|
||||
#
|
||||
%changelog
|
||||
* Fri Oct 20 2017 Justin M. Forbes <jforbes@fedoraproject.org> - 4.14.0-0.rc5.git4.1
|
||||
- Linux v4.14-rc5-94-g9a27ded2195a
|
||||
|
||||
* Thu Oct 19 2017 Justin M. Forbes <jforbes@fedoraproject.org> - 4.14.0-0.rc5.git3.1
|
||||
- Linux v4.14-rc5-31-g73d3393ada4f
|
||||
|
||||
|
||||
2
sources
2
sources
@ -1,4 +1,4 @@
|
||||
SHA512 (linux-4.13.tar.xz) = a557c2f0303ae618910b7106ff63d9978afddf470f03cb72aa748213e099a0ecd5f3119aea6cbd7b61df30ca6ef3ec57044d524b7babbaabddf8b08b8bafa7d2
|
||||
SHA512 (perf-man-4.13.tar.gz) = 9bcc2cd8e56ec583ed2d8e0b0c88e7a94035a1915e40b3177bb02d6c0f10ddd4df9b097b1f5af59efc624226b613e240ddba8ddc2156f3682f992d5455fc5c03
|
||||
SHA512 (patch-4.14-rc5.xz) = 1b09fa9e2fae3b6ac172b2f130a84c9a1ea7c6ea89e0b799013814216dd0c5ba7eeae5b0abcd7dad289fd695abc5663b5fdd92cb7993729c52c08c538b73ace2
|
||||
SHA512 (patch-4.14-rc5-git3.xz) = 89722c66100c36aaa1fba4b4181cf06b4582852429e701bbd5fbedaa5760fb04efa3b9a12f45672cbbfe089f80c4783caf943dedbe70e40a53400461cff52b63
|
||||
SHA512 (patch-4.14-rc5-git4.xz) = d8d992d3e244c27318807dcf1079f47f4de7db3f5fd695fc1006f78e0120eba3813aadaba4ff9ccbd5c4d5a63c763381eef1db8502b33ec3815fbbb8660c45b8
|
||||
|
||||
Loading…
Reference in New Issue
Block a user