| 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: <CAGXu5jKt=S3dHemXgO2nc_adG8cq+h4SsAfaNu9vXxWG_bG1jQ@mail.gmail.com>
Date: Fri, 5 Feb 2016 12:10:01 -0800
From: Kees Cook <keescook@...omium.org>
To: John Stultz <john.stultz@...aro.org>
Cc: lkml <linux-kernel@...r.kernel.org>,
Ruchi Kandoi <kandoiruchi@...gle.com>,
Arjan van de Ven <arjan@...ux.intel.com>,
Thomas Gleixner <tglx@...utronix.de>,
Oren Laadan <orenl@...lrox.com>,
Rom Lemarchand <romlem@...roid.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Android Kernel Team <kernel-team@...roid.com>
Subject: Re: [PATCH] prctl: Add PR_SET_TIMERSLACK_PID for setting timer slack
of an arbitrary thread.
On Fri, Feb 5, 2016 at 10:08 AM, John Stultz <john.stultz@...aro.org> wrote:
> From: Ruchi Kandoi <kandoiruchi@...gle.com>
>
> This allows power/performance management software to set timer
> slack for other threads according to its policy for the thread
> (such as when the thread is designated foreground vs. background
> activity)
>
> Second argument is similar to PR_SET_TIMERSLACK, if non-zero
> then the slack is set to that value otherwise sets it to the
> default for the thread.
>
> Takes PID of the thread as the third argument.
>
> This interface checks that the calling task has permissions to
> to use PTRACE_MODE_ATTACH on the target task, so that we can
> ensure arbitrary apps do not change the timer slack for other
> apps.
>
> Additional fixes from Ruchi and Micha Kalfon <micha@...lrox.com>
> have been folded into this patch to make it easier to reivew.
>
> Cc: Arjan van de Ven <arjan@...ux.intel.com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Oren Laadan <orenl@...lrox.com>
> Cc: Ruchi Kandoi <kandoiruchi@...gle.com>
> Cc: Rom Lemarchand <romlem@...roid.com>
> Cc: Kees Cook <keescook@...omium.org>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Cc: Android Kernel Team <kernel-team@...roid.com>
> Signed-off-by: Ruchi Kandoi <kandoiruchi@...gle.com>
> [jstultz:
> * Folded in CAP_SYS_NICE check from Ruchi.
> * Folded in fix misplaced PR_SET_TIMERSLACK_PID case fix from
> Micha.
> * Folded in make PR_SET_TIMERSLACK_PID pid namespace aware fix
> from Micha.
> * Changed PR_SET_TIMERSLACK_PID so it didn't collide with
> already upstream prctrl values.
> * Reworked commit message.
> * Moved from CAP_SYS_NICE to PTRACE_MODE_ATTACH for permissions
> checks]
> Acked-by: Arjan van de Ven <arjan@...ux.intel.com>
> Reviewed-by: Thomas Gleixner <tglx@...utronix.de>
> Signed-off-by: John Stultz <john.stultz@...aro.org>
> ---
> include/uapi/linux/prctl.h | 7 +++++++
> kernel/sys.c | 25 +++++++++++++++++++++++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index a8d0759..1a13c2b 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -187,6 +187,13 @@ struct prctl_mm_map {
>
> #define PR_SET_FP_MODE 45
> #define PR_GET_FP_MODE 46
> +
> +/* Sets the timerslack for arbitrary threads
Multiline comments should start with a blank:
/*
* Sets the timerslack...
* ...
*/
> + * arg2 slack value, 0 means "use default"
> + * arg3 pid of the thread whose timer slack needs to be set
> + */
> +#define PR_SET_TIMERSLACK_PID 47
> +
> # define PR_FP_MODE_FR (1 << 0) /* 64b FP registers */
> # define PR_FP_MODE_FRE (1 << 1) /* 32b compatibility */
>
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 78947de..5189378 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -41,6 +41,9 @@
> #include <linux/syscore_ops.h>
> #include <linux/version.h>
> #include <linux/ctype.h>
> +#include <linux/mm.h>
> +#include <linux/mempolicy.h>
> +#include <linux/sched.h>
>
> #include <linux/compat.h>
> #include <linux/syscalls.h>
> @@ -2076,6 +2079,7 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> unsigned long, arg4, unsigned long, arg5)
> {
> struct task_struct *me = current;
> + struct task_struct *tsk;
> unsigned char comm[sizeof(me->comm)];
> long error;
>
> @@ -2218,6 +2222,27 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> case PR_GET_TID_ADDRESS:
> error = prctl_get_tid_address(me, (int __user **)arg2);
> break;
> + case PR_SET_TIMERSLACK_PID:
You should reject unused params that are non-zero to allow future
extensions. (Which begs the question, are there other timer-related
things that might need setting too?)
> + rcu_read_lock();
> + tsk = find_task_by_vpid((pid_t)arg3);
> + if (tsk == NULL) {
> + rcu_read_unlock();
> + return -EINVAL;
Traditionally, unable to look up a pid results in ESRCH.
> + }
> + get_task_struct(tsk);
> + rcu_read_unlock();
> + if (ptrace_may_access(tsk, PTRACE_MODE_ATTACH)) {
> + put_task_struct(tsk);
> + return -EPERM;
> + }
> + if (arg2 <= 0)
> + tsk->timer_slack_ns =
> + tsk->default_timer_slack_ns;
> + else
> + tsk->timer_slack_ns = arg2;
> + put_task_struct(tsk);
> + error = 0;
> + break;
> case PR_SET_CHILD_SUBREAPER:
> me->signal->is_child_subreaper = !!arg2;
> break;
> --
> 1.9.1
>
-Kees
--
Kees Cook
Chrome OS & Brillo Security
Powered by blists - more mailing lists