From f3bef67992e8698897b584616535803887c4a73e Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Mon, 23 Nov 2015 16:07:41 -0500 Subject: selinux: fix bug in conditional rules handling commit fa1aa143ac4a ("selinux: extended permissions for ioctls") introduced a bug into the handling of conditional rules, skipping the processing entirely when the caller does not provide an extended permissions (xperms) structure. Access checks from userspace using /sys/fs/selinux/access do not include such a structure since that interface does not presently expose extended permission information. As a result, conditional rules were being ignored entirely on userspace access requests, producing denials when access was allowed by conditional rules in the policy. Fix the bug by only skipping computation of extended permissions in this situation, not the entire conditional rules processing. Reported-by: Laurent Bigonville Signed-off-by: Stephen Smalley [PM: fixed long lines in patch description] Cc: stable@vger.kernel.org # 4.3 Signed-off-by: Paul Moore --- security/selinux/ss/conditional.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index 18643bf9894d..456e1a9bcfde 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -638,7 +638,7 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key, { struct avtab_node *node; - if (!ctab || !key || !avd || !xperms) + if (!ctab || !key || !avd) return; for (node = avtab_search_node(ctab, key); node; @@ -657,7 +657,7 @@ void cond_compute_av(struct avtab *ctab, struct avtab_key *key, if ((u16)(AVTAB_AUDITALLOW|AVTAB_ENABLED) == (node->key.specified & (AVTAB_AUDITALLOW|AVTAB_ENABLED))) avd->auditallow |= node->datum.u.data; - if ((node->key.specified & AVTAB_ENABLED) && + if (xperms && (node->key.specified & AVTAB_ENABLED) && (node->key.specified & AVTAB_XPERMS)) services_compute_xperms_drivers(xperms, node); } -- cgit v1.2.3 From 096fe9eaea40a17e125569f9e657e34cdb6d73bd Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 24 Nov 2015 21:36:31 +0000 Subject: KEYS: Fix handling of stored error in a negatively instantiated user key If a user key gets negatively instantiated, an error code is cached in the payload area. A negatively instantiated key may be then be positively instantiated by updating it with valid data. However, the ->update key type method must be aware that the error code may be there. The following may be used to trigger the bug in the user key type: keyctl request2 user user "" @u keyctl add user user "a" @u which manifests itself as: BUG: unable to handle kernel paging request at 00000000ffffff8a IP: [] __call_rcu.constprop.76+0x1f/0x280 kernel/rcu/tree.c:3046 PGD 7cc30067 PUD 0 Oops: 0002 [#1] SMP Modules linked in: CPU: 3 PID: 2644 Comm: a.out Not tainted 4.3.0+ #49 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 task: ffff88003ddea700 ti: ffff88003dd88000 task.ti: ffff88003dd88000 RIP: 0010:[] [] __call_rcu.constprop.76+0x1f/0x280 [] __call_rcu.constprop.76+0x1f/0x280 kernel/rcu/tree.c:3046 RSP: 0018:ffff88003dd8bdb0 EFLAGS: 00010246 RAX: 00000000ffffff82 RBX: 0000000000000000 RCX: 0000000000000001 RDX: ffffffff81e3fe40 RSI: 0000000000000000 RDI: 00000000ffffff82 RBP: ffff88003dd8bde0 R08: ffff88007d2d2da0 R09: 0000000000000000 R10: 0000000000000000 R11: ffff88003e8073c0 R12: 00000000ffffff82 R13: ffff88003dd8be68 R14: ffff88007d027600 R15: ffff88003ddea700 FS: 0000000000b92880(0063) GS:ffff88007fd00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b CR2: 00000000ffffff8a CR3: 000000007cc5f000 CR4: 00000000000006e0 Stack: ffff88003dd8bdf0 ffffffff81160a8a 0000000000000000 00000000ffffff82 ffff88003dd8be68 ffff88007d027600 ffff88003dd8bdf0 ffffffff810a39e5 ffff88003dd8be20 ffffffff812a31ab ffff88007d027600 ffff88007d027620 Call Trace: [] kfree_call_rcu+0x15/0x20 kernel/rcu/tree.c:3136 [] user_update+0x8b/0xb0 security/keys/user_defined.c:129 [< inline >] __key_update security/keys/key.c:730 [] key_create_or_update+0x291/0x440 security/keys/key.c:908 [< inline >] SYSC_add_key security/keys/keyctl.c:125 [] SyS_add_key+0x101/0x1e0 security/keys/keyctl.c:60 [] entry_SYSCALL_64_fastpath+0x12/0x6a arch/x86/entry/entry_64.S:185 Note the error code (-ENOKEY) in EDX. A similar bug can be tripped by: keyctl request2 trusted user "" @u keyctl add trusted user "a" @u This should also affect encrypted keys - but that has to be correctly parameterised or it will fail with EINVAL before getting to the bit that will crashes. Reported-by: Dmitry Vyukov Signed-off-by: David Howells Acked-by: Mimi Zohar Signed-off-by: James Morris --- security/keys/encrypted-keys/encrypted.c | 2 ++ security/keys/trusted.c | 5 ++++- security/keys/user_defined.c | 5 ++++- 3 files changed, 10 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c index 927db9f35ad6..696ccfa08d10 100644 --- a/security/keys/encrypted-keys/encrypted.c +++ b/security/keys/encrypted-keys/encrypted.c @@ -845,6 +845,8 @@ static int encrypted_update(struct key *key, struct key_preparsed_payload *prep) size_t datalen = prep->datalen; int ret = 0; + if (test_bit(KEY_FLAG_NEGATIVE, &key->flags)) + return -ENOKEY; if (datalen <= 0 || datalen > 32767 || !prep->data) return -EINVAL; diff --git a/security/keys/trusted.c b/security/keys/trusted.c index 903dace648a1..16dec53184b6 100644 --- a/security/keys/trusted.c +++ b/security/keys/trusted.c @@ -1007,13 +1007,16 @@ static void trusted_rcu_free(struct rcu_head *rcu) */ static int trusted_update(struct key *key, struct key_preparsed_payload *prep) { - struct trusted_key_payload *p = key->payload.data[0]; + struct trusted_key_payload *p; struct trusted_key_payload *new_p; struct trusted_key_options *new_o; size_t datalen = prep->datalen; char *datablob; int ret = 0; + if (test_bit(KEY_FLAG_NEGATIVE, &key->flags)) + 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 28cb30f80256..8705d79b2c6f 100644 --- a/security/keys/user_defined.c +++ b/security/keys/user_defined.c @@ -120,7 +120,10 @@ int user_update(struct key *key, struct key_preparsed_payload *prep) if (ret == 0) { /* attach the new data, displacing the old */ - zap = key->payload.data[0]; + if (!test_bit(KEY_FLAG_NEGATIVE, &key->flags)) + zap = key->payload.data[0]; + else + zap = NULL; rcu_assign_keypointer(key, upayload); key->expiry = 0; } -- cgit v1.2.3