Re: [RFC] [Stacking v4 2/3] New version of SELinux patch to support stacking

From: Serge Hallyn (serue@private)
Date: Thu Dec 16 2004 - 15:00:53 PST


The attached patch is virtually the same as Chris', compiled and
"tested".  A quick while (1) malloc(100000000) test seemed to give the
desired result.  Is that an appopriate test?

If so, are there any objections to the patch?

thanks,
-serge

On Wed, 2004-12-08 at 12:21 -0800, Chris Wright wrote:
> * Stephen Smalley (sds@private) wrote:
> > Hmm...maintaining selinux_vm_enough_memory() is a bit of a pain, as we
> > just want the same logic as cap_vm_enough_memory() except for
> > distinguishing the capable() call so that we don't audit CAP_SYS_ADMIN
> > attempts on every process for no reason.  I had originally only
> > suggested replacing the capable() call with a hook call and leaving the
> > rest of vm_enough_memory in the core kernel, but Alan Cox had suggested
> > moving the entire logic into a security hook so that security modules
> > could in the future implement role-based policies on memory allocation
> > (see
> > http://marc.theaimsgroup.com/?l=linux-security-module&m=105638662108785&w=2).
> > That still seems like a good idea, but it would be nice if we could
> > leverage cap_vm_enough_memory or a common helper to reduce the code
> > duplication between it and selinux_vm_enough_memory.
> 
> I agree, there's no point in carrying duplicate logic.  Why don't you
> add a flag for cap_sys_admin set, and change current logic to accpet
> that flag.  Something like this (not compiled, not tested, would need at
> least a header addition).
> 
> ===== security/commoncap.c 1.12 vs edited =====
> --- 1.12/security/commoncap.c	2004-10-25 12:47:50 -07:00
> +++ edited/security/commoncap.c	2004-12-08 12:22:21 -08:00
> @@ -327,7 +327,7 @@ int cap_syslog (int type)
>   * Strict overcommit modes added 2002 Feb 26 by Alan Cox.
>   * Additional code 2002 Jul 20 by Robert Love.
>   */
> -int cap_vm_enough_memory(long pages)
> +int __cap_vm_enough_memory(long pages, int cap_sys_admin)
>  {
>  	unsigned long free, allowed;
>  
> @@ -356,7 +356,7 @@ int cap_vm_enough_memory(long pages)
>  		/*
>  		 * Leave the last 3% for root
>  		 */
> -		if (!capable(CAP_SYS_ADMIN))
> +		if (!cap_sys_admin)
>  			free -= free / 32;
>  
>  		if (free > pages)
> @@ -367,7 +367,7 @@ int cap_vm_enough_memory(long pages)
>  		 * only call if we're about to fail.
>  		 */
>  		n = nr_free_pages();
> -		if (!capable(CAP_SYS_ADMIN))
> +		if (!cap_sys_admin)
>  			n -= n / 32;
>  		free += n;
>  
> @@ -382,7 +382,7 @@ int cap_vm_enough_memory(long pages)
>  	/*
>  	 * Leave the last 3% for root
>  	 */
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!cap_sys_admin)
>  		allowed -= allowed / 32;
>  	allowed += total_swap_pages;
>  
> @@ -394,6 +394,10 @@ int cap_vm_enough_memory(long pages)
>  	return -ENOMEM;
>  }
>  
> +int cap_vm_enough_memory(long pages) {
> +	return __cap_vm_enough_memory(pages, capable(CAP_SYS_ADMIN));
> +}
> +
>  EXPORT_SYMBOL(cap_capable);
>  EXPORT_SYMBOL(cap_settime);
>  EXPORT_SYMBOL(cap_ptrace);
> @@ -409,6 +413,7 @@ EXPORT_SYMBOL(cap_task_post_setuid);
>  EXPORT_SYMBOL(cap_task_reparent_to_init);
>  EXPORT_SYMBOL(cap_syslog);
>  EXPORT_SYMBOL(cap_vm_enough_memory);
> +EXPORT_SYMBOL(__cap_vm_enough_memory);
>  
>  MODULE_DESCRIPTION("Standard Linux Common Capabilities Security Module");
>  MODULE_LICENSE("GPL");
> ===== security/selinux/hooks.c 1.71 vs edited =====
> --- 1.71/security/selinux/hooks.c	2004-11-21 22:42:29 -08:00
> +++ edited/security/selinux/hooks.c	2004-12-08 12:21:20 -08:00
> @@ -1542,62 +1542,17 @@ static int selinux_syslog(int type)
>   */
>  static int selinux_vm_enough_memory(long pages)
>  {
> -	unsigned long free, allowed;
>  	int rc;
>  	struct task_security_struct *tsec = current->security;
>  
> -	vm_acct_memory(pages);
> -
> -        /*
> -	 * Sometimes we want to use more memory than we have
> -	 */
> -	if (sysctl_overcommit_memory == OVERCOMMIT_ALWAYS)
> -		return 0;
> -
> -	if (sysctl_overcommit_memory == OVERCOMMIT_GUESS) {
> -		free = get_page_cache_size();
> -		free += nr_free_pages();
> -		free += nr_swap_pages;
> -
> -		/*
> -		 * Any slabs which are created with the
> -		 * SLAB_RECLAIM_ACCOUNT flag claim to have contents
> -		 * which are reclaimable, under pressure.  The dentry
> -		 * cache and most inode caches should fall into this
> -		 */
> -		free += atomic_read(&slab_reclaim_pages);
> -
> -		/*
> -		 * Leave the last 3% for privileged processes.
> -		 * Don't audit the check, as it is applied to all processes
> -		 * that allocate mappings.
> -		 */
> -		rc = secondary_ops->capable(current, CAP_SYS_ADMIN);
> -		if (!rc) {
> -			rc = avc_has_perm_noaudit(tsec->sid, tsec->sid,
> -						  SECCLASS_CAPABILITY,
> -						  CAP_TO_MASK(CAP_SYS_ADMIN),
> -						  NULL, NULL);
> -		}
> -		if (rc)
> -			free -= free / 32;
> -
> -		if (free > pages)
> -			return 0;
> -		vm_unacct_memory(pages);
> -		return -ENOMEM;
> +	rc = secondary_ops->capable(current, CAP_SYS_ADMIN);
> +	if (!rc) {
> +		rc = avc_has_perm_noaudit(tsec->sid, tsec->sid,
> +					  SECCLASS_CAPABILITY,
> +					  CAP_TO_MASK(CAP_SYS_ADMIN),
> +					  NULL, NULL);
>  	}
> -
> -	allowed = (totalram_pages - hugetlb_total_pages())
> -		* sysctl_overcommit_ratio / 100;
> -	allowed += total_swap_pages;
> -
> -	if (atomic_read(&vm_committed_space) < allowed)
> -		return 0;
> -
> -	vm_unacct_memory(pages);
> -
> -	return -ENOMEM;
> +	return __cap_vm_enough_memory(pages, rc);
>  }
>  
>  /* binprm security operations */
> 
-- 
Serge Hallyn <serue@private>





This archive was generated by hypermail 2.1.3 : Thu Dec 16 2004 - 14:20:00 PST