From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753388AbaCFDvB (ORCPT ); Wed, 5 Mar 2014 22:51:01 -0500 Received: from tundra.namei.org ([65.99.196.166]:48699 "EHLO tundra.namei.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752727AbaCFDu7 (ORCPT ); Wed, 5 Mar 2014 22:50:59 -0500 Date: Thu, 6 Mar 2014 14:51:30 +1100 (EST) From: James Morris To: David Howells cc: torvalds@linux-foundation.org, tt.rantala@gmail.com, keyrings@linux-nfs.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, trinity@vger.kernel.org, Dave Jones Subject: Re: [PATCH] KEYS: Make the keyring cycle detector ignore other keyrings of the same name In-Reply-To: <15425.1393949449@warthog.procyon.org.uk> Message-ID: References: <15425.1393949449@warthog.procyon.org.uk> User-Agent: Alpine 2.02 (LRH 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Acked-by: James Morris On Tue, 4 Mar 2014, David Howells wrote: > > This fixes CVE-2014-0102. > > The following command sequence produces an oops: > > keyctl new_session > i=`keyctl newring _ses @s` > keyctl link @s $i > > The problem is that search_nested_keyrings() sees two keyrings that have > matching type and description, so keyring_compare_object() returns true. > s_n_k() then passes the key to the iterator function - > keyring_detect_cycle_iterator() - which *should* check to see whether this is > the keyring of interest, not just one with the same name. > > Because assoc_array_find() will return one and only one match, I assumed that > the iterator function would only see an exact match or never be called - but > the iterator isn't only called from assoc_array_find()... > > The oops looks something like this: > > kernel BUG at /data/fs/linux-2.6-fscache/security/keys/keyring.c:1003! > invalid opcode: 0000 [#1] SMP > ... > RIP: 0010:[] keyring_detect_cycle_iterator+0xe/0x1f > ... > Stack: > ... > Call Trace: > [] search_nested_keyrings+0x76/0x2aa > [] ? kmem_cache_alloc_trace+0xdd/0x159 > [] ? assoc_array_insert+0xab/0x929 > [] ? selinux_key_permission+0x2d/0x2f > [] ? security_key_permission+0x11/0x13 > [] __key_link_check_live_key+0x50/0x5f > [] ? keyring_describe+0x7b/0x7b > [] key_link+0x4e/0x85 > [] keyctl_keyring_link+0x60/0x81 > [] SyS_keyctl+0x65/0xe4 > [] tracesys+0xdd/0xe2 > > The fix is to make keyring_detect_cycle_iterator() check that the key it has > is the key it was actually looking for rather than calling BUG_ON(). > > A testcase has been included in the keyutils testsuite for this: > > http://git.kernel.org/cgit/linux/kernel/git/dhowells/keyutils.git/commit/?id=891f3365d07f1996778ade0e3428f01878a1790b > > Reported-by: Tommi Rantala > Signed-off-by: David Howells > --- > diff --git a/security/keys/keyring.c b/security/keys/keyring.c > index d46cbc5e335e..2fb2576dc644 100644 > --- a/security/keys/keyring.c > +++ b/security/keys/keyring.c > @@ -1000,7 +1000,11 @@ static int keyring_detect_cycle_iterator(const void *object, > > kenter("{%d}", key->serial); > > - BUG_ON(key != ctx->match_data); > + /* We might get a keyring with matching index-key that is nonetheless a > + * different keyring. */ > + if (key != ctx->match_data) > + return 0; > + > ctx->result = ERR_PTR(-EDEADLK); > return 1; > } > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- James Morris