Re: gfp flags for security_inode_alloc()?

[Casey Schaufler <casey@schaufler-ca.com>]

On 3/29/2012 12:19 AM, Tetsuo Handa wrote:
> Dave Chinner wrote:
>> Yes, because you have no idea what the calling context is except
>> for the fact that is from somewhere inside filesystem code and the
>> filesystem could be holding locks. Therefore, GFP_NOFS is really the
>> only really safe way to allocate memory here.
> I see. Thank you.
>
> I'm not sure, but can call trace happen where somewhere inside network
> filesystem or stackable filesystem code with locks held invokes operations that
> involves GFP_KENREL memory allocation outside that filesystem?
> ----------
> [PATCH] SMACK: Fix incorrect GFP_KERNEL usage.
>
> new_inode_smack() which can be called from smack_inode_alloc_security() needs
> to use GFP_NOFS like SELinux's inode_alloc_security() does, for
> security_inode_alloc() is called from inode_init_always() and
> inode_init_always() is called from xfs_inode_alloc() which is using GFP_NOFS.
>
> smack_inode_init_security() needs to use GFP_NOFS like
> selinux_inode_init_security() does, for initxattrs() callback function (e.g.
> btrfs_initxattrs()) which is called from security_inode_init_security() is
> using GFP_NOFS.
>
> smack_audit_rule_match() needs to use GFP_ATOMIC, for
> security_audit_rule_match() can be called from audit_filter_user_rules() and
> audit_filter_user_rules() is called from audit_filter_user() with RCU read lock
> held.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Acked-by: Casey Schaufler <casey@schaufler-ca.com>

Applied after minor context adjustment to

git://gitorious.org/smack-next/kernel.git


>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index cd667b4..697cf85 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -79,7 +79,7 @@ struct inode_smack *new_inode_smack(char *smack)
>  {
>  	struct inode_smack *isp;
>  
> -	isp = kzalloc(sizeof(struct inode_smack), GFP_KERNEL);
> +	isp = kzalloc(sizeof(struct inode_smack), GFP_NOFS);
>  	if (isp == NULL)
>  		return NULL;
>  
> @@ -562,7 +562,7 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
>  	int may;
>  
>  	if (name) {
> -		*name = kstrdup(XATTR_SMACK_SUFFIX, GFP_KERNEL);
> +		*name = kstrdup(XATTR_SMACK_SUFFIX, GFP_NOFS);
>  		if (*name == NULL)
>  			return -ENOMEM;
>  	}
> @@ -582,7 +582,7 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
>  		    smk_inode_transmutable(dir))
>  			isp = dsp;
>  
> -		*value = kstrdup(isp, GFP_KERNEL);
> +		*value = kstrdup(isp, GFP_NOFS);
>  		if (*value == NULL)
>  			return -ENOMEM;
>  	}
> @@ -3384,7 +3384,7 @@ static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule,
>  	char *rule = vrule;
>  
>  	if (!rule) {
> -		audit_log(actx, GFP_KERNEL, AUDIT_SELINUX_ERR,
> +		audit_log(actx, GFP_ATOMIC, AUDIT_SELINUX_ERR,
>  			  "Smack: missing rule\n");
>  		return -ENOENT;
>  	}
>