Bug 591891 - Backport of keyrings patches upstream since 2.6.32
Summary: Backport of keyrings patches upstream since 2.6.32
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: kernel
Version: 6.0
Hardware: All
OS: Linux
Target Milestone: rc
: ---
Assignee: David Howells
QA Contact: Mike Gahagan
Depends On:
TreeView+ depends on / blocked
Reported: 2010-05-13 12:46 UTC by David Howells
Modified: 2010-11-11 16:03 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2010-11-11 16:03:02 UTC
Target Upstream Version:

Attachments (Terms of Use)

Description David Howells 2010-05-13 12:46:28 UTC
Backport keyrings patches from upstream that have gone since the RHEL-6 diverged from the 2.6.32 vanilla kernel.

commit 7faf2fd8cc3f61dfa4bff8e46d27425636af27a0
Author: David Howells <dhowells@redhat.com>
Date:   Thu May 13 12:49:33 2010 +0100

    KEYS: call_sbin_request_key() must write lock keyrings before modifying them
    call_sbin_request_key() creates a keyring and then attempts to insert a link to
    the authorisation key into that keyring, but does so without holding a write
    lock on the keyring semaphore.
    It will normally get away with this because it hasn't told anyone that the
    keyring exists yet.  The new keyring, however, has had its serial number
    published, which means it can be accessed directly by that handle.
    This was found by a previous patch that adds RCU lockdep checks to the code
    that reads the keyring payload pointer, which includes a check that the keyring
    semaphore is actually locked.
    Without this patch, the following command:
    	keyctl request2 user b a @s
    will provoke the following lockdep warning is displayed in dmesg:
    	[ INFO: suspicious rcu_dereference_check() usage. ]
    	security/keys/keyring.c:727 invoked rcu_dereference_check() without protection!
    	other info that might help us debug this:
    	rcu_scheduler_active = 1, debug_locks = 0
    	2 locks held by keyctl/2076:
    	 #0:  (key_types_sem){.+.+.+}, at: [<ffffffff811a5b29>] key_type_lookup+0x1c/0x71
    	 #1:  (keyring_serialise_link_sem){+.+.+.}, at: [<ffffffff811a6d1e>] __key_link+0x4d/0x3c5
    	stack backtrace:
    	Pid: 2076, comm: keyctl Not tainted 2.6.34-rc6-cachefs #54
    	Call Trace:
    	 [<ffffffff81051fdc>] lockdep_rcu_dereference+0xaa/0xb2
    	 [<ffffffff811a6d1e>] ? __key_link+0x4d/0x3c5
    	 [<ffffffff811a6e6f>] __key_link+0x19e/0x3c5
    	 [<ffffffff811a5952>] ? __key_instantiate_and_link+0xb1/0xdc
    	 [<ffffffff811a59bf>] ? key_instantiate_and_link+0x42/0x5f
    	 [<ffffffff811aa0dc>] call_sbin_request_key+0xe7/0x33b
    	 [<ffffffff8139376a>] ? mutex_unlock+0x9/0xb
    	 [<ffffffff811a5952>] ? __key_instantiate_and_link+0xb1/0xdc
    	 [<ffffffff811a59bf>] ? key_instantiate_and_link+0x42/0x5f
    	 [<ffffffff811aa6fa>] ? request_key_auth_new+0x1c2/0x23c
    	 [<ffffffff810aaf15>] ? cache_alloc_debugcheck_after+0x108/0x173
    	 [<ffffffff811a9d00>] ? request_key_and_link+0x146/0x300
    	 [<ffffffff810ac568>] ? kmem_cache_alloc+0xe1/0x118
    	 [<ffffffff811a9e45>] request_key_and_link+0x28b/0x300
    	 [<ffffffff811a89ac>] sys_request_key+0xf7/0x14a
    	 [<ffffffff81052c0b>] ? trace_hardirqs_on_caller+0x10c/0x130
    	 [<ffffffff81394fb9>] ? trace_hardirqs_on_thunk+0x3a/0x3f
    	 [<ffffffff81001eeb>] system_call_fastpath+0x16/0x1b
    Signed-off-by: David Howells <dhowells@redhat.com>
    Signed-off-by: James Morris <jmorris@namei.org>

commit 7025f3b6636e61a0db150c1161063465151e3251
Author: David Howells <dhowells@redhat.com>
Date:   Thu May 13 12:49:28 2010 +0100

    KEYS: Use RCU dereference wrappers in keyring key type code
    The keyring key type code should use RCU dereference wrappers, even when it
    holds the keyring's key semaphore.
    Reported-by: Vegard Nossum <vegard.nossum@gmail.com>
    Signed-off-by: David Howells <dhowells@redhat.com>
    Acked-by: Serge Hallyn <serue@us.ibm.com>
    Signed-off-by: James Morris <jmorris@namei.org>

commit da727ee644d3d949e8581e095102afcef66ba395
Author: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
Date:   Thu May 13 12:47:01 2010 +0100

    KEYS: find_keyring_by_name() can gain access to a freed keyring
    find_keyring_by_name() can gain access to a keyring that has had its reference
    count reduced to zero, and is thus ready to be freed.  This then allows the
    dead keyring to be brought back into use whilst it is being destroyed.
    The following timeline illustrates the process:
    |(cleaner)                           (user)
    | free_user(user)                    sys_keyctl()
    |  |                                  |
    |  key_put(user->session_keyring)     keyctl_get_keyring_ID()
    |  ||	//=> keyring->usage = 0        |
    |  |schedule_work(&key_cleanup_task)   lookup_user_key()
    |  ||                                   |
    |  kmem_cache_free(,user)               |
    |  .                                    |[KEY_SPEC_USER_KEYRING]
    |  .                                    install_user_keyrings()
    |  .                                    ||
    | key_cleanup() [<= worker_thread()]    ||
    |  |                                    ||
    |  [spin_lock(&key_serial_lock)]        |[mutex_lock(&key_user_keyr..mutex)]
    |  |                                    ||
    |  atomic_read() == 0                   ||
    |  |{ rb_ease(&key->serial_node,) }     ||
    |  |                                    ||
    |  [spin_unlock(&key_serial_lock)]      |find_keyring_by_name()
    |  |                                    |||
    |  keyring_destroy(keyring)             ||[read_lock(&keyring_name_lock)]
    |  ||                                   |||
    |  |[write_lock(&keyring_name_lock)]    ||atomic_inc(&keyring->usage)
    |  |.                                   ||| *** GET freeing keyring ***
    |  |.                                   ||[read_unlock(&keyring_name_lock)]
    |  ||                                   ||
    |  |list_del()                          |[mutex_unlock(&key_user_k..mutex)]
    |  ||                                   |
    |  |[write_unlock(&keyring_name_lock)]  ** INVALID keyring is returned **
    |  |                                    .
    |  kmem_cache_free(,keyring)            .
    |                                       .
    |                                       atomic_dec(&keyring->usage)
    v                                         *** DESTROYED ***
    If CONFIG_SLUB_DEBUG=y then we may see the following message generated:
    	BUG key_jar: Poison overwritten
    	INFO: 0xffff880197a7e200-0xffff880197a7e200. First byte 0x6a instead of 0x6b
    	INFO: Allocated in key_alloc+0x10b/0x35f age=25 cpu=1 pid=5086
    	INFO: Freed in key_cleanup+0xd0/0xd5 age=12 cpu=1 pid=10
    	INFO: Slab 0xffffea000592cb90 objects=16 used=2 fp=0xffff880197a7e200 flags=0x200000000000c3
    	INFO: Object 0xffff880197a7e200 @offset=512 fp=0xffff880197a7e300
    	Bytes b4 0xffff880197a7e1f0:  5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
    	  Object 0xffff880197a7e200:  6a 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b jkkkkkkkkkkkkkkk
    Alternatively, we may see a system panic happen, such as:
    	BUG: unable to handle kernel NULL pointer dereference at 0000000000000001
    	IP: [<ffffffff810e61a3>] kmem_cache_alloc+0x5b/0xe9
    	PGD 6b2b4067 PUD 6a80d067 PMD 0
    	Oops: 0000 [#1] SMP
    	last sysfs file: /sys/kernel/kexec_crash_loaded
    	CPU 1
    	Pid: 31245, comm: su Not tainted 2.6.34-rc5-nofixed-nodebug #2 D2089/PRIMERGY
    	RIP: 0010:[<ffffffff810e61a3>]  [<ffffffff810e61a3>] kmem_cache_alloc+0x5b/0xe9
    	RSP: 0018:ffff88006af3bd98  EFLAGS: 00010002
    	RAX: 0000000000000000 RBX: 0000000000000001 RCX: ffff88007d19900b
    	RDX: 0000000100000000 RSI: 00000000000080d0 RDI: ffffffff81828430
    	RBP: ffffffff81828430 R08: ffff88000a293750 R09: 0000000000000000
    	R10: 0000000000000001 R11: 0000000000100000 R12: 00000000000080d0
    	R13: 00000000000080d0 R14: 0000000000000296 R15: ffffffff810f20ce
    	FS:  00007f97116bc700(0000) GS:ffff88000a280000(0000) knlGS:0000000000000000
    	CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    	CR2: 0000000000000001 CR3: 000000006a91c000 CR4: 00000000000006e0
    	DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
    	DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
    	Process su (pid: 31245, threadinfo ffff88006af3a000, task ffff8800374414c0)
    	 0000000512e0958e 0000000000008000 ffff880037f8d180 0000000000000001
    	 0000000000000000 0000000000008001 ffff88007d199000 ffffffff810f20ce
    	 0000000000008000 ffff88006af3be48 0000000000000024 ffffffff810face3
    	Call Trace:
    	 [<ffffffff810f20ce>] ? get_empty_filp+0x70/0x12f
    	 [<ffffffff810face3>] ? do_filp_open+0x145/0x590
    	 [<ffffffff810ce208>] ? tlb_finish_mmu+0x2a/0x33
    	 [<ffffffff810ce43c>] ? unmap_region+0xd3/0xe2
    	 [<ffffffff810e4393>] ? virt_to_head_page+0x9/0x2d
    	 [<ffffffff81103916>] ? alloc_fd+0x69/0x10e
    	 [<ffffffff810ef4ed>] ? do_sys_open+0x56/0xfc
    	 [<ffffffff81008a02>] ? system_call_fastpath+0x16/0x1b
    	Code: 0f 1f 44 00 00 49 89 c6 fa 66 0f 1f 44 00 00 65 4c 8b 04 25 60 e8 00 00 48 8b 45 00 49 01 c0 49 8b 18 48 85 db 74 0d 48 63 45 18 <48> 8b 04 03 49 89 00 eb 14 4c 89 f9 83 ca ff 44 89 e6 48 89 ef
    	RIP  [<ffffffff810e61a3>] kmem_cache_alloc+0x5b/0xe9
    This problem is that find_keyring_by_name does not confirm that the keyring is
    valid before accepting it.
    Skipping keyrings that have been reduced to a zero count seems the way to go.
    To this end, use atomic_inc_not_zero() to increment the usage count and skip
    the candidate keyring if that returns false.
    The following script _may_ cause the bug to happen, but there's no guarantee
    as the window of opportunity is small:
    	/bin/su -c "exit;" $USER || { /usr/sbin/adduser -m $USER; add=1; }
    	for ((i=0; i<LOOP; i++))
    		/bin/su -c "echo '$i' > /dev/null" $USER
    	(( add == 1 )) && /usr/sbin/userdel -r $USER
    Note that the nominated user must not be in use.
    An alternative way of testing this may be:
    	for ((i=0; i<100000; i++))
    		keyctl session foo /bin/true || break
    	done >&/dev/null
    as that uses a keyring named "foo" rather than relying on the user and
    user-session named keyrings.
    Reported-by: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
    Signed-off-by: David Howells <dhowells@redhat.com>
    Tested-by: Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>
    Acked-by: Serge Hallyn <serue@us.ibm.com>
    Signed-off-by: James Morris <jmorris@namei.org>

commit e022c44bc759db6bae815f90abe4c94b00320a69
Author: David Howells <dhowells@redhat.com>
Date:   Thu May 13 12:46:37 2010 +0100

    KEYS: Fix RCU handling in key_gc_keyring()
    key_gc_keyring() needs to either hold the RCU read lock or hold the keyring
    semaphore if it's going to scan the keyring's list.  Given that it only needs
    to read the key list, and it's doing so under a spinlock, the RCU read lock is
    the thing to use.
    Furthermore, the RCU check added in e7b0a61b7929632d36cf052d9e2820ef0a9c1bfe is
    incorrect as holding the spinlock on key_serial_lock is not grounds for
    assuming a keyring's pointer list can be read safely.  Instead, a simple
    rcu_dereference() inside of the previously mentioned RCU read lock is what we
    Reported-by: Serge E. Hallyn <serue@us.ibm.com>
    Signed-off-by: David Howells <dhowells@redhat.com>
    Acked-by: Serge Hallyn <serue@us.ibm.com>
    Acked-by: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
    Signed-off-by: James Morris <jmorris@namei.org>

commit 9770b005fb87d5186064465c5396a91f325a15e2
Author: David Howells <dhowells@redhat.com>
Date:   Thu May 13 12:44:19 2010 +0100

    keys: the request_key() syscall should link an existing key to the dest keyring
    The request_key() system call and request_key_and_link() should make a
    link from an existing key to the destination keyring (if supplied), not
    just from a new key to the destination keyring.
    This can be tested by:
    	ring=`keyctl newring fred @s`
    	keyctl request2 user debug:a a
    	keyctl request user debug:a $ring
    	keyctl list $ring
    If it says:
    	keyring is empty
    then it didn't work.  If it shows something like:
    	1 key in keyring:
    	1070462727: --alswrv     0     0 user: debug:a
    then it did.
    request_key() system call is meant to recursively search all your keyrings for
    the key you desire, and, optionally, if it doesn't exist, call out to userspace
    to create one for you.
    If request_key() finds or creates a key, it should, optionally, create a link
    to that key from the destination keyring specified.
    Therefore, if, after a successful call to request_key() with a desination
    keyring specified, you see the destination keyring empty, the code didn't work
    If you see the found key in the keyring, then it did - which is what the patch
    is required for.
    Signed-off-by: David Howells <dhowells@redhat.com>
    Cc: James Morris <jmorris@namei.org>
    Cc: <stable@kernel.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

commit ca8a3e003ec17e1ab182c98a30b73b7805f0e9c0
Author: David Howells <dhowells@redhat.com>
Date:   Thu May 13 12:43:34 2010 +0100

    keys: don't need to use RCU in keyring_read() as semaphore is held
    keyring_read() doesn't need to use rcu_dereference() to access the keyring
    payload as the caller holds the key semaphore to prevent modifications
    from happening whilst the data is read out.
    This should solve the following warning:
    [ INFO: suspicious rcu_dereference_check() usage. ]
    security/keys/keyring.c:204 invoked rcu_dereference_check() without protection!
    other info that might help us debug this:
    rcu_scheduler_active = 1, debug_locks = 0
    1 lock held by keyctl/2144:
     #0:  (&key->sem){+++++.}, at: [<ffffffff81177f7c>] keyctl_read_key+0x9c/0xcf
    stack backtrace:
    Pid: 2144, comm: keyctl Not tainted 2.6.34-rc2-cachefs #113
    Call Trace:
     [<ffffffff8105121f>] lockdep_rcu_dereference+0xaa/0xb2
     [<ffffffff811762d5>] keyring_read+0x4d/0xe7
     [<ffffffff81177f8c>] keyctl_read_key+0xac/0xcf
     [<ffffffff811788d4>] sys_keyctl+0x75/0xb9
     [<ffffffff81001eeb>] system_call_fastpath+0x16/0x1b
    Signed-off-by: David Howells <dhowells@redhat.com>
    Cc: Herbert Xu <herbert@gondor.apana.org.au>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: James Morris <jmorris@namei.org>

commit 01493bfde761e3d64d842230e6acc85a068e3d0f
Author: David Howells <dhowells@redhat.com>
Date:   Thu May 13 12:42:48 2010 +0100

    keys: fix an RCU warning
    Fix the following RCU warning:
      [ INFO: suspicious rcu_dereference_check() usage. ]
      security/keys/request_key.c:116 invoked rcu_dereference_check() without protection!
    This was caused by doing:
    	[root@andromeda ~]# keyctl newring fred @s
    	[root@andromeda ~]# keyctl request2 user a a 539196288
    	request_key: Required key not available
    Signed-off-by: David Howells <dhowells@redhat.com>
    Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

commit f80983f50a0f1e0bdc21b17005a233d49aefe3fd
Author: Roel Kluin <roel.kluin@gmail.com>
Date:   Thu May 13 12:39:25 2010 +0100

    keys: PTR_ERR return of wrong pointer in keyctl_get_security()
    Return the PTR_ERR of the correct pointer.
    Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Acked-by: David Howells <dhowells@redhat.com>
    Signed-off-by: James Morris <jmorris@namei.org>

Comment 2 RHEL Program Management 2010-05-13 14:06:36 UTC
This request was evaluated by Red Hat Product Management for inclusion in a Red
Hat Enterprise Linux major release.  Product Management has requested further
review of this request by Red Hat Engineering, for potential inclusion in a Red
Hat Enterprise Linux Major release.  This request is not yet committed for

Comment 4 David Howells 2010-05-20 14:20:54 UTC
An additional patch queued for upstream:

Author: Dan Carpenter <error27@gmail.com>
Date:   Thu May 20 15:17:49 2010 +0100

KEYS: Return more accurate error codes
We were using the wrong variable here so the error codes weren't being returned
properly.  The original code returns -ENOKEY.
Signed-off-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>

Comment 5 Aristeu Rozanski 2010-05-28 20:37:30 UTC
Patch(es) available on kernel-2.6.32-31.el6

Comment 9 Aristeu Rozanski 2010-07-01 16:20:20 UTC
Patch(es) available on kernel-2.6.32-42.el6

Comment 12 Mike Gahagan 2010-08-25 17:39:59 UTC
confirmed functionality of updated keyrings support with the keyutils testsuite in Snap 12, test suite has also run in multiple previous snapshots.

Comment 13 releng-rhel@redhat.com 2010-11-11 16:03:02 UTC
Red Hat Enterprise Linux 6.0 is now available and should resolve
the problem described in this bug report. This report is therefore being closed
with a resolution of CURRENTRELEASE. You may reopen this bug report if the
solution does not work for you.

Note You need to log in before you can comment on or make changes to this bug.