| Message ID | 1479882559-34854-1-git-send-email-himanshu.sh@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/22/2016 10:29 PM, Himanshu Shukla wrote: > There is race condition issue while freeing the i_security blob in SMACK > module. There is existing condition where i_security can be freed while > inode_permission is called from path lookup on second CPU. There has been > observed the page fault with such condition. VFS code and Selinux module > takes care of this condition by freeing the inode and i_security field > using RCU via call_rcu(). But in SMACK directly the i_secuirty blob is > being freed. Use call_rcu() to fix this race condition issue. > > Signed-off-by: Himanshu Shukla <himanshu.sh@samsung.com> > Signed-off-by: Vishal Goel <vishal.goel@samsung.com> Acked-by: Casey Schaufler <casey@schaufler-ca.com> I have queued this for 4.11 as it's too late for 4.10. Minor change noted inline. > --- > security/smack/smack.h | 1 + > security/smack/smack_lsm.c | 32 ++++++++++++++++++++++++++++---- > 2 files changed, 29 insertions(+), 4 deletions(-) > > diff --git a/security/smack/smack.h b/security/smack/smack.h > index 51fd301..22f816e 100644 > --- a/security/smack/smack.h > +++ b/security/smack/smack.h > @@ -114,6 +114,7 @@ struct inode_smack { > struct smack_known *smk_mmap; /* label of the mmap domain */ > struct mutex smk_lock; /* initialization lock */ > int smk_flags; /* smack inode flags */ > + struct rcu_head rcu; /* for freeing the inode_smack */ Used smk_rcu instead of rcu. > }; > > struct task_smack { > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index 1cb0602..67f8708 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -1009,15 +1009,39 @@ static int smack_inode_alloc_security(struct inode *inode) > } > > /** > - * smack_inode_free_security - free an inode blob > + * smack_inode_free_rcu - Free inode_smack blob from cache > + * @head: the rcu_head for getting inode_smack pointer > + * > + * Call back function called from call_rcu() to free > + * the i_security blob pointer in inode > + */ > +static void smack_inode_free_rcu(struct rcu_head *head) > +{ > + struct inode_smack *issp; > + > + issp = container_of(head, struct inode_smack, rcu); > + kmem_cache_free(smack_inode_cache, issp); > +} > + > +/** > + * smack_inode_free_security - free an inode blob using call_rcu() > * @inode: the inode with a blob > * > - * Clears the blob pointer in inode > + * Clears the blob pointer in inode using RCU > */ > static void smack_inode_free_security(struct inode *inode) > { > - kmem_cache_free(smack_inode_cache, inode->i_security); > - inode->i_security = NULL; > + struct inode_smack *issp = inode->i_security; > + > + /* > + * The inode may still be referenced in a path walk and > + * a call to smack_inode_permission() can be made > + * after smack_inode_free_security() is called. > + * To avoid race condition free the i_security via RCU > + * and leave the current inode->i_security pointer intact. > + * The inode will be freed after the RCU grace period too. > + */ > + call_rcu(&issp->rcu, smack_inode_free_rcu); > } > > /** -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/security/smack/smack.h b/security/smack/smack.h index 51fd301..22f816e 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -114,6 +114,7 @@ struct inode_smack { struct smack_known *smk_mmap; /* label of the mmap domain */ struct mutex smk_lock; /* initialization lock */ int smk_flags; /* smack inode flags */ + struct rcu_head rcu; /* for freeing the inode_smack */ }; struct task_smack { diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 1cb0602..67f8708 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1009,15 +1009,39 @@ static int smack_inode_alloc_security(struct inode *inode) } /** - * smack_inode_free_security - free an inode blob + * smack_inode_free_rcu - Free inode_smack blob from cache + * @head: the rcu_head for getting inode_smack pointer + * + * Call back function called from call_rcu() to free + * the i_security blob pointer in inode + */ +static void smack_inode_free_rcu(struct rcu_head *head) +{ + struct inode_smack *issp; + + issp = container_of(head, struct inode_smack, rcu); + kmem_cache_free(smack_inode_cache, issp); +} + +/** + * smack_inode_free_security - free an inode blob using call_rcu() * @inode: the inode with a blob * - * Clears the blob pointer in inode + * Clears the blob pointer in inode using RCU */ static void smack_inode_free_security(struct inode *inode) { - kmem_cache_free(smack_inode_cache, inode->i_security); - inode->i_security = NULL; + struct inode_smack *issp = inode->i_security; + + /* + * The inode may still be referenced in a path walk and + * a call to smack_inode_permission() can be made + * after smack_inode_free_security() is called. + * To avoid race condition free the i_security via RCU + * and leave the current inode->i_security pointer intact. + * The inode will be freed after the RCU grace period too. + */ + call_rcu(&issp->rcu, smack_inode_free_rcu); } /**