summaryrefslogtreecommitdiff
path: root/security
AgeCommit message (Collapse)Author
2014-02-10Fix install_process_keyring error handlingAndi Kleen
commit 27d6379894be4a81984da4d48002196a83939ca9 upstream. Fix an incorrect error check that returns 1 for error instead of the expected error code. Signed-off-by: Andi Kleen <ak@linux.intel.com> Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
2014-02-10keys: fix race with concurrent install_user_keyrings()David Howells
commit 0da9dfdd2cd9889201bc6f6f43580c99165cd087 upstream. This fixes CVE-2013-1792. There is a race in install_user_keyrings() that can cause a NULL pointer dereference when called concurrently for the same user if the uid and uid-session keyrings are not yet created. It might be possible for an unprivileged user to trigger this by calling keyctl() from userspace in parallel immediately after logging in. Assume that we have two threads both executing lookup_user_key(), both looking for KEY_SPEC_USER_SESSION_KEYRING. THREAD A THREAD B =============================== =============================== ==>call install_user_keyrings(); if (!cred->user->session_keyring) ==>call install_user_keyrings() ... user->uid_keyring = uid_keyring; if (user->uid_keyring) return 0; <== key = cred->user->session_keyring [== NULL] user->session_keyring = session_keyring; atomic_inc(&key->usage); [oops] At the point thread A dereferences cred->user->session_keyring, thread B hasn't updated user->session_keyring yet, but thread A assumes it is populated because install_user_keyrings() returned ok. The race window is really small but can be exploited if, for example, thread B is interrupted or preempted after initializing uid_keyring, but before doing setting session_keyring. This couldn't be reproduced on a stock kernel. However, after placing systemtap probe on 'user->session_keyring = session_keyring;' that introduced some delay, the kernel could be crashed reliably. Fix this by checking both pointers before deciding whether to return. Alternatively, the test could be done away with entirely as it is checked inside the mutex - but since the mutex is global, that may not be the best way. Signed-off-by: David Howells <dhowells@redhat.com> Reported-by: Mateusz Guzik <mguzik@redhat.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: James Morris <james.l.morris@oracle.com> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
2012-08-17KEYS: Fix a NULL pointer deref in the user-defined key typeDavid Howells
commit 9f35a33b8d06263a165efe3541d9aa0cdbd70b3b upstream. Fix a NULL pointer deref in the user-defined key type whereby updating a negative key into a fully instantiated key will cause an oops to occur when the code attempts to free the non-existent old payload. This results in an oops that looks something like the following: BUG: unable to handle kernel NULL pointer dereference at 0000000000000008 IP: [<ffffffff81085fa1>] __call_rcu+0x11/0x13e PGD 3391d067 PUD 3894a067 PMD 0 Oops: 0002 [#1] SMP CPU 1 Pid: 4354, comm: keyctl Not tainted 3.1.0-fsdevel+ #1140 /DG965RY RIP: 0010:[<ffffffff81085fa1>] [<ffffffff81085fa1>] __call_rcu+0x11/0x13e RSP: 0018:ffff88003d591df8 EFLAGS: 00010246 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 000000000000006e RDX: ffffffff8161d0c0 RSI: 0000000000000000 RDI: 0000000000000000 RBP: ffff88003d591e18 R08: 0000000000000000 R09: ffffffff8152fa6c R10: 0000000000000000 R11: 0000000000000300 R12: ffff88003b8f9538 R13: ffffffff8161d0c0 R14: ffff88003b8f9d50 R15: ffff88003c69f908 FS: 00007f97eb18c720(0000) GS:ffff88003bd00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000008 CR3: 000000003d47a000 CR4: 00000000000006e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Process keyctl (pid: 4354, threadinfo ffff88003d590000, task ffff88003c78a040) Stack: ffff88003e0ffde0 ffff88003b8f9538 0000000000000001 ffff88003b8f9d50 ffff88003d591e28 ffffffff810860f0 ffff88003d591e68 ffffffff8117bfea ffff88003d591e68 ffffffff00000000 ffff88003e0ffde1 ffff88003e0ffde0 Call Trace: [<ffffffff810860f0>] call_rcu_sched+0x10/0x12 [<ffffffff8117bfea>] user_update+0x8d/0xa2 [<ffffffff8117723a>] key_create_or_update+0x236/0x270 [<ffffffff811789b1>] sys_add_key+0x123/0x17e [<ffffffff813b84bb>] system_call_fastpath+0x16/0x1b Signed-off-by: David Howells <dhowells@redhat.com> Acked-by: Jeff Layton <jlayton@redhat.com> Acked-by: Neil Horman <nhorman@redhat.com> Acked-by: Steve Dickson <steved@redhat.com> Acked-by: James Morris <jmorris@namei.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> [PG: pre-3.0 kernels dont have kfree_rcu, so tweak accordingly] Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
2012-08-17fcaps: clear the same personality flags as suid when fcaps are usedEric Paris
commit d52fc5dde171f030170a6cb78034d166b13c9445 upstream. If a process increases permissions using fcaps all of the dangerous personality flags which are cleared for suid apps should also be cleared. Thus programs given priviledge with fcaps will continue to have address space randomization enabled even if the parent tried to disable it to make it easier to attack. Signed-off-by: Eric Paris <eparis@redhat.com> Reviewed-by: Serge Hallyn <serge.hallyn@canonical.com> Signed-off-by: James Morris <james.l.morris@oracle.com> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
2012-08-17security: fix compile error in commoncap.cJonghwan Choi
commit 51b79bee627d526199b2f6a6bef8ee0c0739b6d1 upstream. Add missing "personality.h" security/commoncap.c: In function 'cap_bprm_set_creds': security/commoncap.c:510: error: 'PER_CLEAR_ON_SETID' undeclared (first use in this function) security/commoncap.c:510: error: (Each undeclared identifier is reported only once security/commoncap.c:510: error: for each function it appears in.) Signed-off-by: Jonghwan Choi <jhbird.choi@samsung.com> Acked-by: Serge Hallyn <serge.hallyn@canonical.com> Signed-off-by: James Morris <james.l.morris@oracle.com> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
2011-06-26CRED: Fix BUG() upon security_cred_alloc_blank() failureTetsuo Handa
commit 2edeaa34a6e3f2c43b667f6c4f7b27944b811695 upstream. In cred_alloc_blank() since 2.6.32, abort_creds(new) is called with new->security == NULL and new->magic == 0 when security_cred_alloc_blank() returns an error. As a result, BUG() will be triggered if SELinux is enabled or CONFIG_DEBUG_CREDENTIALS=y. If CONFIG_DEBUG_CREDENTIALS=y, BUG() is called from __invalid_creds() because cred->magic == 0. Failing that, BUG() is called from selinux_cred_free() because selinux_cred_free() is not expecting cred->security == NULL. This does not affect smack_cred_free(), tomoyo_cred_free() or apparmor_cred_free(). Fix these bugs by (1) Set new->magic before calling security_cred_alloc_blank(). (2) Handle null cred->security in creds_are_invalid() and selinux_cred_free(). Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
2011-06-26SELinux: do not compute transition labels on mountpoint labeled filesystemsEric Paris
commit 415103f9932d45f7927f4b17e3a9a13834cdb9a1 upstream. selinux_inode_init_security computes transitions sids even for filesystems that use mount point labeling. It shouldn't do that. It should just use the mount point label always and no matter what. This causes 2 problems. 1) it makes file creation slower than it needs to be since we calculate the transition sid and 2) it allows files to be created with a different label than the mount point! # id -Z staff_u:sysadm_r:sysadm_t:s0-s0:c0.c1023 # sesearch --type --class file --source sysadm_t --target tmp_t Found 1 semantic te rules: type_transition sysadm_t tmp_t : file user_tmp_t; # mount -o loop,context="system_u:object_r:tmp_t:s0" /tmp/fs /mnt/tmp # ls -lZ /mnt/tmp drwx------. root root system_u:object_r:tmp_t:s0 lost+found # touch /mnt/tmp/file1 # ls -lZ /mnt/tmp -rw-r--r--. root root staff_u:object_r:user_tmp_t:s0 file1 drwx------. root root system_u:object_r:tmp_t:s0 lost+found Whoops, we have a mount point labeled filesystem tmp_t with a user_tmp_t labeled file! Signed-off-by: Eric Paris <eparis@redhat.com> Reviewed-by: Reviewed-by: James Morris <jmorris@namei.org> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
2011-06-26SELinux: define permissions for DCB netlink messagesEric Paris
commit 350e4f31e0eaf56dfc3b328d24a11bdf42a41fb8 upstream. Commit 2f90b865 added two new netlink message types to the netlink route socket. SELinux has hooks to define if netlink messages are allowed to be sent or received, but it did not know about these two new message types. By default we allow such actions so noone likely noticed. This patch adds the proper definitions and thus proper permissions enforcement. Signed-off-by: Eric Paris <eparis@redhat.com> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
2011-04-17ima: fix add LSM rule bugMimi Zohar
commit 867c20265459d30a01b021a9c1e81fb4c5832aa9 upstream. If security_filter_rule_init() doesn't return a rule, then not everything is as fine as the return code implies. This bug only occurs when the LSM (eg. SELinux) is disabled at runtime. Adding an empty LSM rule causes ima_match_rules() to always succeed, ignoring any remaining rules. default IMA TCB policy: # PROC_SUPER_MAGIC dont_measure fsmagic=0x9fa0 # SYSFS_MAGIC dont_measure fsmagic=0x62656572 # DEBUGFS_MAGIC dont_measure fsmagic=0x64626720 # TMPFS_MAGIC dont_measure fsmagic=0x01021994 # SECURITYFS_MAGIC dont_measure fsmagic=0x73636673 < LSM specific rule > dont_measure obj_type=var_log_t measure func=BPRM_CHECK measure func=FILE_MMAP mask=MAY_EXEC measure func=FILE_CHECK mask=MAY_READ uid=0 Thus without the patch, with the boot parameters 'tcb selinux=0', adding the above 'dont_measure obj_type=var_log_t' rule to the default IMA TCB measurement policy, would result in nothing being measured. The patch prevents the default TCB policy from being replaced. Signed-off-by: Mimi Zohar <zohar@us.ibm.com> Cc: James Morris <jmorris@namei.org> Acked-by: Serge Hallyn <serge.hallyn@canonical.com> Cc: David Safford <safford@watson.ibm.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
2011-01-06KEYS: Fix bug in keyctl_session_to_parent() if parent has no session keyringDavid Howells
commit 3d96406c7da1ed5811ea52a3b0905f4f0e295376 upstream. Fix a bug in keyctl_session_to_parent() whereby it tries to check the ownership of the parent process's session keyring whether or not the parent has a session keyring [CVE-2010-2960]. This results in the following oops: BUG: unable to handle kernel NULL pointer dereference at 00000000000000a0 IP: [<ffffffff811ae4dd>] keyctl_session_to_parent+0x251/0x443 ... Call Trace: [<ffffffff811ae2f3>] ? keyctl_session_to_parent+0x67/0x443 [<ffffffff8109d286>] ? __do_fault+0x24b/0x3d0 [<ffffffff811af98c>] sys_keyctl+0xb4/0xb8 [<ffffffff81001eab>] system_call_fastpath+0x16/0x1b if the parent process has no session keyring. If the system is using pam_keyinit then it mostly protected against this as all processes derived from a login will have inherited the session keyring created by pam_keyinit during the log in procedure. To test this, pam_keyinit calls need to be commented out in /etc/pam.d/. Reported-by: Tavis Ormandy <taviso@cmpxchg8b.com> Signed-off-by: David Howells <dhowells@redhat.com> Acked-by: Tavis Ormandy <taviso@cmpxchg8b.com> Cc: dann frazier <dannf@debian.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
2011-01-06KEYS: Fix RCU no-lock warning in keyctl_session_to_parent()David Howells
commit 9d1ac65a9698513d00e5608d93fca0c53f536c14 upstream. There's an protected access to the parent process's credentials in the middle of keyctl_session_to_parent(). This results in the following RCU warning: =================================================== [ INFO: suspicious rcu_dereference_check() usage. ] --------------------------------------------------- security/keys/keyctl.c:1291 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-session-/2137: #0: (tasklist_lock){.+.+..}, at: [<ffffffff811ae2ec>] keyctl_session_to_parent+0x60/0x236 stack backtrace: Pid: 2137, comm: keyctl-session- Not tainted 2.6.36-rc2-cachefs+ #1 Call Trace: [<ffffffff8105606a>] lockdep_rcu_dereference+0xaa/0xb3 [<ffffffff811ae379>] keyctl_session_to_parent+0xed/0x236 [<ffffffff811af77e>] sys_keyctl+0xb4/0xb6 [<ffffffff81001eab>] system_call_fastpath+0x16/0x1b The code should take the RCU read lock to make sure the parents credentials don't go away, even though it's holding a spinlock and has IRQ disabled. Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
2011-01-06KEYS: Return more accurate error codesDan Carpenter
commit 4d09ec0f705cf88a12add029c058b53f288cfaa2 upstream. 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> Signed-off-by: James Morris <jmorris@namei.org> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
2010-05-14mmap_min_addr check CAP_SYS_RAWIO only for writeKees Cook
Redirecting directly to lsm, here's the patch discussed on lkml: http://lkml.org/lkml/2010/4/22/219 The mmap_min_addr value is useful information for an admin to see without being root ("is my system vulnerable to kernel NULL pointer attacks?") and its setting is trivially easy for an attacker to determine by calling mmap() in PAGE_SIZE increments starting at 0, so trying to keep it private has no value. Only require CAP_SYS_RAWIO if changing the value, not reading it. Comment from Serge : Me, I like to write my passwords with light blue pen on dark blue paper, pasted on my window - if you're going to get my password, you're gonna get a headache. Signed-off-by: Kees Cook <kees.cook@canonical.com> Acked-by: Serge Hallyn <serue@us.ibm.com> Signed-off-by: James Morris <jmorris@namei.org> (cherry picked from commit 822cceec7248013821d655545ea45d1c6a9d15b3)
2010-05-05KEYS: call_sbin_request_key() must write lock keyrings before modifying themDavid Howells
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>
2010-05-05KEYS: Use RCU dereference wrappers in keyring key type codeDavid Howells
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>
2010-05-05KEYS: find_keyring_by_name() can gain access to a freed keyringToshiyuki Okajima
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 *** TIME 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) Stack: 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/sh LOOP=100000 USER=dummy_user /bin/su -c "exit;" $USER || { /usr/sbin/adduser -m $USER; add=1; } for ((i=0; i<LOOP; i++)) do /bin/su -c "echo '$i' > /dev/null" $USER done (( add == 1 )) && /usr/sbin/userdel -r $USER exit Note that the nominated user must not be in use. An alternative way of testing this may be: for ((i=0; i<100000; i++)) do 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>
2010-05-05KEYS: Fix RCU handling in key_gc_keyring()David Howells
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 want. 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>
2010-05-05KEYS: Fix an RCU warning in the reading of user keysDavid Howells
Fix an RCU warning in the reading of user keys: =================================================== [ INFO: suspicious rcu_dereference_check() usage. ] --------------------------------------------------- security/keys/user_defined.c:202 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/3637: #0: (&key->sem){+++++.}, at: [<ffffffff811a80ae>] keyctl_read_key+0x9c/0xcf stack backtrace: Pid: 3637, comm: keyctl Not tainted 2.6.34-rc5-cachefs #18 Call Trace: [<ffffffff81051f6c>] lockdep_rcu_dereference+0xaa/0xb2 [<ffffffff811aa55f>] user_read+0x47/0x91 [<ffffffff811a80be>] keyctl_read_key+0xac/0xcf [<ffffffff811a8a06>] sys_keyctl+0x75/0xb7 [<ffffffff81001eeb>] system_call_fastpath+0x16/0x1b Signed-off-by: David Howells <dhowells@redhat.com> Acked-by: Serge Hallyn <serue@us.ibm.com> Signed-off-by: James Morris <jmorris@namei.org>
2010-04-27Merge branch 'for-linus' of ↵Linus Torvalds
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6 * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6: keys: don't need to use RCU in keyring_read() as semaphore is held
2010-04-27keys: the request_key() syscall should link an existing key to the dest keyringDavid Howells
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 correctly. 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>
2010-04-28keys: don't need to use RCU in keyring_read() as semaphore is heldDavid Howells
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>
2010-04-24keys: fix an RCU warningDavid Howells
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 539196288 [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>
2010-04-22security: testing the wrong variable in create_by_name()Dan Carpenter
There is a typo here. We should be testing "*dentry" instead of "dentry". If "*dentry" is an ERR_PTR, it gets dereferenced in either mkdir() or create() which would cause an OOPs. Signed-off-by: Dan Carpenter <error27@gmail.com> Signed-off-by: James Morris <jmorris@namei.org>
2010-04-15SELinux: Reduce max avtab size to avoid page allocation failuresStephen Smalley
Reduce MAX_AVTAB_HASH_BITS so that the avtab allocation is an order 2 allocation rather than an order 4 allocation on x86_64. This addresses reports of page allocation failures: http://marc.info/?l=selinux&m=126757230625867&w=2 https://bugzilla.redhat.com/show_bug.cgi?id=570433 Reported-by: Russell Coker <russell@coker.com.au> Signed-off-by: Stephen D. Smalley <sds@tycho.nsa.gov> Acked-by: Eric Paris <eparis@redhat.com> Signed-off-by: James Morris <jmorris@namei.org>
2010-03-30include cleanup: Update gfp.h and slab.h includes to prepare for breaking ↵Tejun Heo
implicit slab.h inclusion from percpu.h percpu.h is included by sched.h and module.h and thus ends up being included when building most .c files. percpu.h includes slab.h which in turn includes gfp.h making everything defined by the two files universally available and complicating inclusion dependencies. percpu.h -> slab.h dependency is about to be removed. Prepare for this change by updating users of gfp and slab facilities include those headers directly instead of assuming availability. As this conversion needs to touch large number of source files, the following script is used as the basis of conversion. http://userweb.kernel.org/~tj/misc/slabh-sweep.py The script does the followings. * Scan files for gfp and slab usages and update includes such that only the necessary includes are there. ie. if only gfp is used, gfp.h, if slab is used, slab.h. * When the script inserts a new include, it looks at the include blocks and try to put the new include such that its order conforms to its surrounding. It's put in the include block which contains core kernel includes, in the same order that the rest are ordered - alphabetical, Christmas tree, rev-Xmas-tree or at the end if there doesn't seem to be any matching order. * If the script can't find a place to put a new include (mostly because the file doesn't have fitting include block), it prints out an error message indicating which .h file needs to be added to the file. The conversion was done in the following steps. 1. The initial automatic conversion of all .c files updated slightly over 4000 files, deleting around 700 includes and adding ~480 gfp.h and ~3000 slab.h inclusions. The script emitted errors for ~400 files. 2. Each error was manually checked. Some didn't need the inclusion, some needed manual addition while adding it to implementation .h or embedding .c file was more appropriate for others. This step added inclusions to around 150 files. 3. The script was run again and the output was compared to the edits from #2 to make sure no file was left behind. 4. Several build tests were done and a couple of problems were fixed. e.g. lib/decompress_*.c used malloc/free() wrappers around slab APIs requiring slab.h to be added manually. 5. The script was run on all .h files but without automatically editing them as sprinkling gfp.h and slab.h inclusions around .h files could easily lead to inclusion dependency hell. Most gfp.h inclusion directives were ignored as stuff from gfp.h was usually wildly available and often used in preprocessor macros. Each slab.h inclusion directive was examined and added manually as necessary. 6. percpu.h was updated not to include slab.h. 7. Build test were done on the following configurations and failures were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my distributed build env didn't work with gcov compiles) and a few more options had to be turned off depending on archs to make things build (like ipr on powerpc/64 which failed due to missing writeq). * x86 and x86_64 UP and SMP allmodconfig and a custom test config. * powerpc and powerpc64 SMP allmodconfig * sparc and sparc64 SMP allmodconfig * ia64 SMP allmodconfig * s390 SMP allmodconfig * alpha SMP allmodconfig * um on x86_64 SMP allmodconfig 8. percpu.h modifications were reverted so that it could be applied as a separate patch and serve as bisection point. Given the fact that I had only a couple of failures from tests on step 6, I'm fairly confident about the coverage of this conversion patch. If there is a breakage, it's likely to be something in one of the arch headers which should be easily discoverable easily on most builds of the specific arch. Signed-off-by: Tejun Heo <tj@kernel.org> Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
2010-03-15tomoyo: fix potential use after freeDan Carpenter
The original code returns a freed pointer. This function is expected to return NULL on errors. Signed-off-by: Dan Carpenter <error27@gmail.com> Acked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Signed-off-by: James Morris <jmorris@namei.org>
2010-03-08Merge branch 'for-next' into for-linusJiri Kosina
Conflicts: Documentation/filesystems/proc.txt arch/arm/mach-u300/include/mach/debug-macro.S drivers/net/qlge/qlge_ethtool.c drivers/net/qlge/qlge_main.c drivers/net/typhoon.c
2010-03-04Merge branch 'for-linus' of ↵Linus Torvalds
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6 * 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6: (52 commits) init: Open /dev/console from rootfs mqueue: fix typo "failues" -> "failures" mqueue: only set error codes if they are really necessary mqueue: simplify do_open() error handling mqueue: apply mathematics distributivity on mq_bytes calculation mqueue: remove unneeded info->messages initialization mqueue: fix mq_open() file descriptor leak on user-space processes fix race in d_splice_alias() set S_DEAD on unlink() and non-directory rename() victims vfs: add NOFOLLOW flag to umount(2) get rid of ->mnt_parent in tomoyo/realpath hppfs can use existing proc_mnt, no need for do_kern_mount() in there Mirror MS_KERNMOUNT in ->mnt_flags get rid of useless vfsmount_lock use in put_mnt_ns() Take vfsmount_lock to fs/internal.h get rid of insanity with namespace roots in tomoyo take check for new events in namespace (guts of mounts_poll()) to namespace.c Don't mess with generic_permission() under ->d_lock in hpfs sanitize const/signedness for udf nilfs: sanitize const/signedness in dealing with ->d_name.name ... Fix up fairly trivial (famous last words...) conflicts in drivers/infiniband/core/uverbs_main.c and security/tomoyo/realpath.c
2010-03-03get rid of ->mnt_parent in tomoyo/realpathAl Viro
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
2010-03-03get rid of insanity with namespace roots in tomoyoAl Viro
passing *any* namespace root to __d_path() as root is equivalent to just passing it {NULL, NULL}; no need to bother with finding the root of our namespace in there. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
2010-03-03fix mnt_mountpoint abuse in smackAl Viro
(mnt,mnt_mountpoint) pair is conceptually wrong; if you want to use it for generating pathname and for nothing else *and* if you know that vfsmount tree is unchanging, you can get away with that, but the right solution for that is (mnt,mnt_root). Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
2010-03-01Merge branch 'next' into for-linusJames Morris
2010-02-28Merge branch 'core-rcu-for-linus' of ↵Linus Torvalds
git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip * 'core-rcu-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip: (44 commits) rcu: Fix accelerated GPs for last non-dynticked CPU rcu: Make non-RCU_PROVE_LOCKING rcu_read_lock_sched_held() understand boot rcu: Fix accelerated grace periods for last non-dynticked CPU rcu: Export rcu_scheduler_active rcu: Make rcu_read_lock_sched_held() take boot time into account rcu: Make lockdep_rcu_dereference() message less alarmist sched, cgroups: Fix module export rcu: Add RCU_CPU_STALL_VERBOSE to dump detailed per-task information rcu: Fix rcutorture mod_timer argument to delay one jiffy rcu: Fix deadlock in TREE_PREEMPT_RCU CPU stall detection rcu: Convert to raw_spinlocks rcu: Stop overflowing signed integers rcu: Use canonical URL for Mathieu's dissertation rcu: Accelerate grace period if last non-dynticked CPU rcu: Fix citation of Mathieu's dissertation rcu: Documentation update for CONFIG_PROVE_RCU security: Apply lockdep-based checking to rcu_dereference() uses idr: Apply lockdep-based diagnostics to rcu_dereference() uses radix-tree: Disable RCU lockdep checking in radix tree vfs: Abstract rcu_dereference_check for files-fdtable use ...
2010-02-26SELinux: Make selinux_kernel_create_files_as() shouldn't just always return 0David Howells
Make selinux_kernel_create_files_as() return an error when it gets one, rather than unconditionally returning 0. Without this, cachefiles doesn't return an error if the SELinux policy doesn't let it create files with the label of the directory at the base of the cache. Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: James Morris <jmorris@namei.org>
2010-02-26TOMOYO: Protect find_task_by_vpid() with RCU.Tetsuo Handa
Holding tasklist_lock is no longer sufficient for find_task_by_vpid(). Explicit rcu_read_lock() is required. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> -- security/tomoyo/common.c | 4 ++++ 1 file changed, 4 insertions(+) Signed-off-by: James Morris <jmorris@namei.org>
2010-02-25security: Apply lockdep-based checking to rcu_dereference() usesPaul E. McKenney
Apply lockdep-ified RCU primitives to key_gc_keyring() and keyring_destroy(). Cc: David Howells <dhowells@redhat.com> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Cc: laijs@cn.fujitsu.com Cc: dipankar@in.ibm.com Cc: mathieu.desnoyers@polymtl.ca Cc: josh@joshtriplett.org Cc: dvhltc@us.ibm.com Cc: niv@us.ibm.com Cc: peterz@infradead.org Cc: rostedt@goodmis.org Cc: Valdis.Kletnieks@vt.edu Cc: dhowells@redhat.com LKML-Reference: <1266887105-1528-12-git-send-email-paulmck@linux.vnet.ibm.com> Signed-off-by: Ingo Molnar <mingo@elte.hu>
2010-02-25netlabel: fix export of SELinux categories > 127Joshua Roys
This fixes corrupted CIPSO packets when SELinux categories greater than 127 are used. The bug occured on the second (and later) loops through the while; the inner for loop through the ebitmap->maps array used the same index as the NetLabel catmap->bitmap array, even though the NetLabel bitmap is twice as long as the SELinux bitmap. Signed-off-by: Joshua Roys <joshua.roys@gtri.gatech.edu> Acked-by: Paul Moore <paul.moore@hp.com> Signed-off-by: James Morris <jmorris@namei.org>
2010-02-25security: fix error return path in ima_inode_allocXiaotian Feng
If radix_tree_preload is failed in ima_inode_alloc, we don't need radix_tree_preload_end because kernel is alread preempt enabled Signed-off-by: Xiaotian Feng <dfeng@redhat.com> Signed-off-by: Mimi Zohar <zohar@us.ibm.com> Signed-off-by: James Morris <jmorris@namei.org>
2010-02-24Security: add static to security_ops and default_security_ops variablewzt.wzt@gmail.com
Enhance the security framework to support resetting the active security module. This eliminates the need for direct use of the security_ops and default_security_ops variables outside of security.c, so make security_ops and default_security_ops static. Also remove the secondary_ops variable as a cleanup since there is no use for that. secondary_ops was originally used by SELinux to call the "secondary" security module (capability or dummy), but that was replaced by direct calls to capability and the only remaining use is to save and restore the original security ops pointer value if SELinux is disabled by early userspace based on /etc/selinux/config. Further, if we support this directly in the security framework, then we can just use &default_security_ops for this purpose since that is now available. Signed-off-by: Zhitong Wang <zhitong.wangzt@alibaba-inc.com> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> Signed-off-by: James Morris <jmorris@namei.org>
2010-02-22selinux: libsepol: remove dead code in check_avtab_hierarchy_callback()KaiGai Kohei
This patch revert the commit of 7d52a155e38d5a165759dbbee656455861bf7801 which removed a part of type_attribute_bounds_av as a dead code. However, at that time, we didn't find out the target side boundary allows to handle some of pseudo /proc/<pid>/* entries with its process's security context well. Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> -- security/selinux/ss/services.c | 43 ++++++++++++++++++++++++++++++++++++--- 1 files changed, 39 insertions(+), 4 deletions(-) Signed-off-by: James Morris <jmorris@namei.org>
2010-02-17TOMOYO: Remove __func__ from tomoyo_is_correct_path/domainTetsuo Handa
__func__ is used for only debug printk(). We can remove it. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Signed-off-by: James Morris <jmorris@namei.org>
2010-02-16security: fix a couple of sparse warningsJames Morris
Fix a couple of sparse warnings for callers of context_struct_to_string, which takes a *u32, not an *int. These cases are harmless as the values are not used. Signed-off-by: James Morris <jmorris@namei.org> Acked-by: KaiGai Kohei <kaigai@ak.jp.nec.com>
2010-02-16TOMOYO: Remove unneeded parameter.Tetsuo Handa
tomoyo_path_perm() tomoyo_path2_perm() and tomoyo_check_rewrite_permission() always receive tomoyo_domain(). We can move it from caller to callee. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Signed-off-by: James Morris <jmorris@namei.org>
2010-02-16TOMOYO: Use shorter names.Tetsuo Handa
Use shorter name to reduce newlines needed for 80 columns limit. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Signed-off-by: James Morris <jmorris@namei.org>
2010-02-16TOMOYO: Use enum for index numbers.Tetsuo Handa
Use enum to declare index numbers. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Signed-off-by: James Morris <jmorris@namei.org>
2010-02-15TOMOYO: Add garbage collector.Tetsuo Handa
This patch adds garbage collector support to TOMOYO. Elements are protected by "struct srcu_struct tomoyo_ss". Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Acked-by: Serge Hallyn <serue@us.ibm.com> Signed-off-by: James Morris <jmorris@namei.org>
2010-02-15TOMOYO: Add refcounter on domain structure.Tetsuo Handa
Add refcounter to "struct tomoyo_domain_info" since garbage collector needs to determine whether this struct is referred by "struct cred"->security or not. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Acked-by: Serge Hallyn <serue@us.ibm.com> Signed-off-by: James Morris <jmorris@namei.org>
2010-02-15TOMOYO: Merge headers.Tetsuo Handa
Gather structures and constants scattered around security/tomoyo/ directory. This is for preparation for adding garbage collector since garbage collector needs to know structures and constants which TOMOYO uses. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Acked-by: Serge Hallyn <serue@us.ibm.com> Signed-off-by: James Morris <jmorris@namei.org>
2010-02-15TOMOYO: Add refcounter on string data.Tetsuo Handa
Add refcounter to "struct tomoyo_name_entry" and replace tomoyo_save_name() with tomoyo_get_name()/tomoyo_put_name() pair so that we can kfree() when garbage collector is added. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Acked-by: Serge Hallyn <serue@us.ibm.com> Signed-off-by: James Morris <jmorris@namei.org>
2010-02-11TOMOYO: Reduce lines by using common path for addition and deletion.Tetsuo Handa
Since the codes for adding an entry and removing an entry are similar, we can save some lines by using "if (is_delete) { ... } else { ... }" branches. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Acked-by: Serge Hallyn <serue@us.ibm.com> Signed-off-by: James Morris <jmorris@namei.org>