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