[prev in list] [next in list] [prev in thread] [next in thread]
List: selinux
Subject: Re: [PATCH] selinux: fix inode security list corruption
From: Paul Moore <paul () paul-moore ! com>
Date: 2014-10-06 20:31:35
Message-ID: 10815679.kovQJLmdnJ () sifl
[Download RAW message or body]
On Thursday, October 02, 2014 01:00:25 PM Stephen Smalley wrote:
> 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>
> ---
> security/selinux/hooks.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied, thanks for taking the time to track this down and submit a patch.
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index b0e9404..e03bad5 100644
> --- 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);
--
paul moore
www.paul-moore.com
_______________________________________________
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic