Re: [PATCH] vfs: Fix possible NULL pointer dereference in inode_permission()
From: Paul E. McKenney <hidden>
Date: 2014-01-09 23:45:37
Also in:
linux-fsdevel, lkml
On Thu, Jan 09, 2014 at 06:27:56PM -0500, Steven Rostedt wrote:
quoted hunk ↗ jump to hunk
On Thu, 9 Jan 2014 18:25:23 -0500 Steven Rostedt [off-list ref] wrote:quoted
On Fri, 10 Jan 2014 06:41:03 +0800 Linus Torvalds [off-list ref] wrote:quoted
I think the sane short term fix is to make the kfree() of the i_security member be a rcu free, and not clear the member.You mean my first patch? https://lkml.org/lkml/2014/1/9/349Oh wait, you said not to clear the member. Thus, the patch would look like this: Signed-off-by: Steven Rostedt <rostedt@goodmis.org> Index: linux-trace.git/security/selinux/hooks.c ===================================================================--- linux-trace.git.orig/security/selinux/hooks.c +++ linux-trace.git/security/selinux/hooks.c@@ -234,6 +234,14 @@ static int inode_alloc_security(struct i return 0; } +static void inode_free_rcu(struct rcu_head *head) +{ + struct inode_security_struct *isec; + + isec = container_of(head, struct inode_security_struct, rcu); + kmem_cache_free(sel_inode_cache, isec); +} + static void inode_free_security(struct inode *inode) { struct inode_security_struct *isec = inode->i_security;@@ -244,8 +252,7 @@ static void inode_free_security(struct i list_del_init(&isec->list); spin_unlock(&sbsec->isec_lock); - inode->i_security = NULL; - kmem_cache_free(sel_inode_cache, isec); + call_rcu(&isec->rcu, inode_free_rcu);
Does not clearing ->i_security mean that RCU readers can traverse this pointer after the invocation of call_rcu()? If so, this is problematic. (If something else already prevents readers from getting here, no problem.) Thanx, Paul
quoted hunk ↗ jump to hunk
} static int file_alloc_security(struct file *file) Index: linux-trace.git/security/selinux/include/objsec.h ===================================================================--- linux-trace.git.orig/security/selinux/include/objsec.h +++ linux-trace.git/security/selinux/include/objsec.h@@ -38,7 +38,10 @@ struct task_security_struct { struct inode_security_struct { struct inode *inode; /* back pointer to inode object */ - struct list_head list; /* list of inode_security_struct */ + union { + struct list_head list; /* list of inode_security_struct */ + struct rcu_head rcu; /* for freeing the inode_security_struct */ + }; u32 task_sid; /* SID of creating task */ u32 sid; /* SID of this object */ u16 sclass; /* security class of this object */