From 923190d32de4428afbea5e5773be86bea60a9925 Mon Sep 17 00:00:00 2001 From: Stephen Smalley Date: Mon, 6 Oct 2014 16:32:52 -0400 Subject: [PATCH 1/3] selinux: fix inode security list corruption 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 Signed-off-by: Stephen Smalley Cc: stable@vger.kernel.org Signed-off-by: Paul Moore --- security/selinux/hooks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 29e64d4ca099..2478976fc894 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -481,6 +481,7 @@ static int sb_finish_set_opts(struct super_block *sb) 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 @@ static int sb_finish_set_opts(struct super_block *sb) iput(inode); } spin_lock(&sbsec->isec_lock); - list_del_init(&isec->list); goto next_inode; } spin_unlock(&sbsec->isec_lock); From d950f84c1c6658faec2ecbf5b09f7e7191953394 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Wed, 12 Nov 2014 14:01:34 -0500 Subject: [PATCH 2/3] selinux: convert WARN_ONCE() to printk() in selinux_nlmsg_perm() Convert WARN_ONCE() to printk() in selinux_nlmsg_perm(). After conversion from audit_log() in commit e173fb26, WARN_ONCE() was deemed too alarmist, so switch it to printk(). Signed-off-by: Richard Guy Briggs [PM: Changed to printk(WARNING) so we catch all of the different invalid netlink messages. In Richard's defense, he brought this point up earlier, but I didn't understand his point at the time.] Signed-off-by: Paul Moore --- security/selinux/hooks.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 2478976fc894..654f0710620a 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4727,9 +4727,10 @@ static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb) err = selinux_nlmsg_lookup(sksec->sclass, nlh->nlmsg_type, &perm); if (err) { if (err == -EINVAL) { - WARN_ONCE(1, "selinux_nlmsg_perm: unrecognized netlink message:" - " protocol=%hu nlmsg_type=%hu sclass=%hu\n", - sk->sk_protocol, nlh->nlmsg_type, sksec->sclass); + printk(KERN_WARNING + "SELinux: unrecognized netlink message:" + " protocol=%hu nlmsg_type=%hu sclass=%hu\n", + sk->sk_protocol, nlh->nlmsg_type, sksec->sclass); if (!selinux_enforcing || security_get_allow_unknown()) err = 0; } From 00fec2a10b51a071ec92da256ccd30f6b13fc55b Mon Sep 17 00:00:00 2001 From: Yao Dongdong Date: Fri, 28 Nov 2014 04:25:35 +0000 Subject: [PATCH 3/3] selinux: Remove security_ops extern security_ops is not used in this file. Signed-off-by: Yao Dongdong Signed-off-by: Paul Moore --- security/selinux/hooks.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 654f0710620a..49fc8338bcc7 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -95,8 +95,6 @@ #include "audit.h" #include "avc_ss.h" -extern struct security_operations *security_ops; - /* SECMARK reference count */ static atomic_t selinux_secmark_refcount = ATOMIC_INIT(0);