| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <201401172115.GCC00099.OFQMFFtSLHOOJV@I-love.SAKURA.ne.jp>
Date: Fri, 17 Jan 2014 21:15:34 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: james.l.morris@...cle.com
Cc: linux-kernel@...r.kernel.org, akpm@...ux-foundation.org
Subject: Re: [PATCH(v2)] commoncap: don't alloc the credential unless needed in cap_task_prctl
James, would you pick up this patch?
Tetsuo Handa wrote:
> James, please apply.
>
> Kevin, does "cause OOM" mean something like the OOM killer or kmalloc() failure
> is triggerred by frequent prctl() requests by userspace applications?
> If yes, we should backport to stable as this affects any 2.6.29 and later kernels.
> --------------------
> From 1cb92c00bd1529f5d16a3f13c09f2bd6b6f7c6ca Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
> Date: Thu, 19 Dec 2013 06:23:04 +0900
> Subject: [PATCH] commoncap: don't alloc the credential unless needed in cap_task_prctl
>
> In function cap_task_prctl(), we would allocate a credential
> unconditionally and then check if we support the requested function.
> If not we would release this credential with abort_creds() by using
> RCU method. But on some archs such as powerpc, the sys_prctl is heavily
> used to get/set the floating point exception mode. So the unnecessary
> allocating/releasing of credential not only introduce runtime overhead
> but also do cause OOM due to the RCU implementation.
>
> This patch removes abort_creds() from cap_task_prctl() by calling
> prepare_creds() only when we need to modify it.
>
> Reported-by: Kevin Hao <haokexin@...il.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
> Acked-by: Serge E. Hallyn <serge.hallyn@...ntu.com>
> ---
> security/commoncap.c | 134 +++++++++++++++++++++++++------------------------
> 1 files changed, 68 insertions(+), 66 deletions(-)
>
> diff --git a/security/commoncap.c b/security/commoncap.c
> index b9d613e..2b62f44 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -818,19 +818,75 @@ int cap_task_setnice(struct task_struct *p, int nice)
> return cap_safe_nice(p);
> }
>
> +static int cap_prctl_read(const struct cred *old, unsigned long arg2)
> +{
> + if (!cap_valid(arg2))
> + return -EINVAL;
> + return !!cap_raised(old->cap_bset, arg2);
> +}
> +
> /*
> * Implement PR_CAPBSET_DROP. Attempt to remove the specified capability from
> * the current task's bounding set. Returns 0 on success, -ve on error.
> */
> -static long cap_prctl_drop(struct cred *new, unsigned long cap)
> +static int cap_prctl_drop(const struct cred *old, unsigned long cap)
> {
> - if (!ns_capable(current_user_ns(), CAP_SETPCAP))
> + struct cred *new;
> +
> + if (!ns_capable(old->user_ns, CAP_SETPCAP))
> return -EPERM;
> if (!cap_valid(cap))
> return -EINVAL;
> -
> + new = prepare_creds();
> + if (!new)
> + return -ENOMEM;
> cap_lower(new->cap_bset, cap);
> - return 0;
> + return commit_creds(new);
> +}
> +
> +static int cap_prctl_securebits(const struct cred *old, unsigned long arg2)
> +{
> + struct cred *new;
> +
> + if ((((old->securebits & SECURE_ALL_LOCKS) >> 1)
> + & (old->securebits ^ arg2)) /*[1]*/
> + || ((old->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/
> + || (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/
> + || (cap_capable(old, old->user_ns, CAP_SETPCAP,
> + SECURITY_CAP_AUDIT) != 0) /*[4]*/
> + /*
> + * [1] no changing of bits that are locked
> + * [2] no unlocking of locks
> + * [3] no setting of unsupported bits
> + * [4] doing anything requires privilege (go read about
> + * the "sendmail capabilities bug")
> + */
> + )
> + /* cannot change a locked bit */
> + return -EPERM;
> + new = prepare_creds();
> + if (!new)
> + return -ENOMEM;
> + new->securebits = arg2;
> + return commit_creds(new);
> +}
> +
> +static int cap_prctl_keepcaps(unsigned long arg2)
> +{
> + struct cred *new;
> +
> + if (arg2 > 1) /* Note, we rely on arg2 being unsigned here */
> + return -EINVAL;
> + if (issecure(SECURE_KEEP_CAPS_LOCKED))
> + return -EPERM;
> + new = prepare_creds();
> + if (!new)
> + return -ENOMEM;
> + if (arg2)
> + new->securebits |= issecure_mask(SECURE_KEEP_CAPS);
> + else
> + new->securebits &= ~issecure_mask(SECURE_KEEP_CAPS);
> + return commit_creds(new);
> }
>
> /**
> @@ -848,26 +904,14 @@ static long cap_prctl_drop(struct cred *new, unsigned long cap)
> int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> unsigned long arg4, unsigned long arg5)
> {
> - struct cred *new;
> - long error = 0;
> -
> - new = prepare_creds();
> - if (!new)
> - return -ENOMEM;
> + const struct cred *old = current_cred();
>
> switch (option) {
> case PR_CAPBSET_READ:
> - error = -EINVAL;
> - if (!cap_valid(arg2))
> - goto error;
> - error = !!cap_raised(new->cap_bset, arg2);
> - goto no_change;
> + return cap_prctl_read(old, arg2);
>
> case PR_CAPBSET_DROP:
> - error = cap_prctl_drop(new, arg2);
> - if (error < 0)
> - goto error;
> - goto changed;
> + return cap_prctl_drop(old, arg2);
>
> /*
> * The next four prctl's remain to assist with transitioning a
> @@ -889,63 +933,21 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
> * capability-based-privilege environment.
> */
> case PR_SET_SECUREBITS:
> - error = -EPERM;
> - if ((((new->securebits & SECURE_ALL_LOCKS) >> 1)
> - & (new->securebits ^ arg2)) /*[1]*/
> - || ((new->securebits & SECURE_ALL_LOCKS & ~arg2)) /*[2]*/
> - || (arg2 & ~(SECURE_ALL_LOCKS | SECURE_ALL_BITS)) /*[3]*/
> - || (cap_capable(current_cred(),
> - current_cred()->user_ns, CAP_SETPCAP,
> - SECURITY_CAP_AUDIT) != 0) /*[4]*/
> - /*
> - * [1] no changing of bits that are locked
> - * [2] no unlocking of locks
> - * [3] no setting of unsupported bits
> - * [4] doing anything requires privilege (go read about
> - * the "sendmail capabilities bug")
> - */
> - )
> - /* cannot change a locked bit */
> - goto error;
> - new->securebits = arg2;
> - goto changed;
> + return cap_prctl_securebits(old, arg2);
>
> case PR_GET_SECUREBITS:
> - error = new->securebits;
> - goto no_change;
> + return old->securebits;
>
> case PR_GET_KEEPCAPS:
> - if (issecure(SECURE_KEEP_CAPS))
> - error = 1;
> - goto no_change;
> + return !!issecure(SECURE_KEEP_CAPS);
>
> case PR_SET_KEEPCAPS:
> - error = -EINVAL;
> - if (arg2 > 1) /* Note, we rely on arg2 being unsigned here */
> - goto error;
> - error = -EPERM;
> - if (issecure(SECURE_KEEP_CAPS_LOCKED))
> - goto error;
> - if (arg2)
> - new->securebits |= issecure_mask(SECURE_KEEP_CAPS);
> - else
> - new->securebits &= ~issecure_mask(SECURE_KEEP_CAPS);
> - goto changed;
> + return cap_prctl_keepcaps(arg2);
>
> default:
> /* No functionality available - continue with default */
> - error = -ENOSYS;
> - goto error;
> + return -ENOSYS;
> }
> -
> - /* Functionality provided */
> -changed:
> - return commit_creds(new);
> -
> -no_change:
> -error:
> - abort_creds(new);
> - return error;
> }
>
> /**
> --
> 1.7.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists