| From 923190d32de4428afbea5e5773be86bea60a9925 Mon Sep 17 00:00:00 2001 |
| From: Stephen Smalley <sds@tycho.nsa.gov> |
| Date: Mon, 6 Oct 2014 16:32:52 -0400 |
| Subject: selinux: fix inode security list corruption |
| |
| From: Stephen Smalley <sds@tycho.nsa.gov> |
| |
| commit 923190d32de4428afbea5e5773be86bea60a9925 upstream. |
| |
| sb_finish_set_opts() can race with inode_free_security() |
| when initializing inode security structures for inodes |
| created prior to initial policy load or by the filesystem |
| during ->mount(). This appears to have always been |
| a possible race, but commit 3dc91d4 ("SELinux: Fix possible |
| NULL pointer dereference in selinux_inode_permission()") |
| made it more evident by immediately reusing the unioned |
| list/rcu element of the inode security structure for call_rcu() |
| upon an inode_free_security(). But the underlying issue |
| was already present before that commit as a possible use-after-free |
| of isec. |
| |
| Shivnandan Kumar reported the list corruption and proposed |
| a patch to split the list and rcu elements out of the union |
| as separate fields of the inode_security_struct so that setting |
| the rcu element would not affect the list element. However, |
| this would merely hide the issue and not truly fix the code. |
| |
| This patch instead moves up the deletion of the list entry |
| prior to dropping the sbsec->isec_lock initially. Then, |
| if the inode is dropped subsequently, there will be no further |
| references to the isec. |
| |
| Reported-by: Shivnandan Kumar <shivnandan.k@samsung.com> |
| Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> |
| Signed-off-by: Paul Moore <pmoore@redhat.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| security/selinux/hooks.c | 2 +- |
| 1 file changed, 1 insertion(+), 1 deletion(-) |
| |
| --- a/security/selinux/hooks.c |
| +++ b/security/selinux/hooks.c |
| @@ -481,6 +481,7 @@ next_inode: |
| list_entry(sbsec->isec_head.next, |
| struct inode_security_struct, list); |
| struct inode *inode = isec->inode; |
| + list_del_init(&isec->list); |
| spin_unlock(&sbsec->isec_lock); |
| inode = igrab(inode); |
| if (inode) { |
| @@ -489,7 +490,6 @@ next_inode: |
| iput(inode); |
| } |
| spin_lock(&sbsec->isec_lock); |
| - list_del_init(&isec->list); |
| goto next_inode; |
| } |
| spin_unlock(&sbsec->isec_lock); |