Re: 2.6.25-rc8-mm1 - BUG in fs/jbd/transaction.c

[James Morris <jmorris@namei.org>]

On Fri, 4 Apr 2008, Stephen Smalley wrote:

> 
> On Fri, 2008-04-04 at 10:02 +1100, James Morris wrote:
> > On Thu, 3 Apr 2008, Stephen Smalley wrote:
> > 
> > > > > Try this patch, it will keep us from re-entering the fs when we aren't supposed
> > > > > to.  cc'ing Eric Paris since he's the only selinux guy I know :).  I don't think
> > 
> > Please see the MAINTAINERS file.
> > 
> > > > > any of the other allocations in here need to be fixed, but I didn't look too
> > > > > carefully.
> > > > > 
> > > > > Signed-off-by: Josef Bacik <jbacik@redhat.com>
> > 
> > Thanks, I'll push this to Linus, but note that further analysis is 
> > required.
> 
> Please review.

Looks good to me.

> 
> More cases where SELinux must not re-enter the fs code.
> Called from the d_instantiate security hook.
> 
> Signed-off-by:  Stephen Smalley <sds@tycho.nsa.gov>
> 
> ---
> 
>  security/selinux/hooks.c            |    7 ++++---
>  security/selinux/include/security.h |    3 ++-
>  security/selinux/ss/services.c      |   12 +++++++-----
>  3 files changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 41a049f..95b51b6 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1143,7 +1143,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
>  		}
>  
>  		len = INITCONTEXTLEN;
> -		context = kmalloc(len, GFP_KERNEL);
> +		context = kmalloc(len, GFP_NOFS);
>  		if (!context) {
>  			rc = -ENOMEM;
>  			dput(dentry);
> @@ -1161,7 +1161,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
>  			}
>  			kfree(context);
>  			len = rc;
> -			context = kmalloc(len, GFP_KERNEL);
> +			context = kmalloc(len, GFP_NOFS);
>  			if (!context) {
>  				rc = -ENOMEM;
>  				dput(dentry);
> @@ -1185,7 +1185,8 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
>  			rc = 0;
>  		} else {
>  			rc = security_context_to_sid_default(context, rc, &sid,
> -			                                     sbsec->def_sid);
> +							     sbsec->def_sid,
> +							     GFP_NOFS);
>  			if (rc) {
>  				printk(KERN_WARNING "%s:  context_to_sid(%s) "
>  				       "returned %d for dev=%s ino=%ld\n",
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index f7d2f03..44e12ec 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -86,7 +86,8 @@ int security_sid_to_context(u32 sid, char **scontext,
>  int security_context_to_sid(char *scontext, u32 scontext_len,
>  	u32 *out_sid);
>  
> -int security_context_to_sid_default(char *scontext, u32 scontext_len, u32 *out_sid, u32 def_sid);
> +int security_context_to_sid_default(char *scontext, u32 scontext_len,
> +				    u32 *out_sid, u32 def_sid, gfp_t gfp_flags);
>  
>  int security_get_user_sids(u32 callsid, char *username,
>  			   u32 **sids, u32 *nel);
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index f374186..3f2bad2 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -680,7 +680,8 @@ out:
>  
>  }
>  
> -static int security_context_to_sid_core(char *scontext, u32 scontext_len, u32 *sid, u32 def_sid)
> +static int security_context_to_sid_core(char *scontext, u32 scontext_len,
> +					u32 *sid, u32 def_sid, gfp_t gfp_flags)
>  {
>  	char *scontext2;
>  	struct context context;
> @@ -709,7 +710,7 @@ static int security_context_to_sid_core(char *scontext, u32 scontext_len, u32 *s
>  	   null suffix to the copy to avoid problems with the existing
>  	   attr package, which doesn't view the null terminator as part
>  	   of the attribute value. */
> -	scontext2 = kmalloc(scontext_len+1,GFP_KERNEL);
> +	scontext2 = kmalloc(scontext_len+1, gfp_flags);
>  	if (!scontext2) {
>  		rc = -ENOMEM;
>  		goto out;
> @@ -809,7 +810,7 @@ out:
>  int security_context_to_sid(char *scontext, u32 scontext_len, u32 *sid)
>  {
>  	return security_context_to_sid_core(scontext, scontext_len,
> -	                                    sid, SECSID_NULL);
> +					    sid, SECSID_NULL, GFP_KERNEL);
>  }
>  
>  /**
> @@ -829,10 +830,11 @@ int security_context_to_sid(char *scontext, u32 scontext_len, u32 *sid)
>   * Returns -%EINVAL if the context is invalid, -%ENOMEM if insufficient
>   * memory is available, or 0 on success.
>   */
> -int security_context_to_sid_default(char *scontext, u32 scontext_len, u32 *sid, u32 def_sid)
> +int security_context_to_sid_default(char *scontext, u32 scontext_len, u32 *sid,
> +				    u32 def_sid, gfp_t gfp_flags)
>  {
>  	return security_context_to_sid_core(scontext, scontext_len,
> -	                                    sid, def_sid);
> +					    sid, def_sid, gfp_flags);
>  }
>  
>  static int compute_sid_handle_invalid_context(
> 
> 

-- 
James Morris
<jmorris@namei.org>