| 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: <CALAqxLVW_2NUaPwrD0mpRLUh7cnLJpe_s0ctdzHz9x5NpNo=xw@mail.gmail.com>
Date: Fri, 5 Feb 2016 12:18:51 -0800
From: John Stultz <john.stultz@...aro.org>
To: Kees Cook <keescook@...omium.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 12:10 PM, Kees Cook <keescook@...omium.org> wrote:
> 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?)
Ok. Will look into this. As for other timer related things, I don't
think we have any userspace reachable interfaces. There was some
discussion around infinitely deferrable timers (so timers that only
fire if we're already expiring timers, but wouldn't ever cause a hw
event on their own), but that was likely to be done w/ a per-timer
specified flags, and stalled out awhile back. However, I suspect folks
might eventually want a similar ability to stop a specific task from
causing any wakeups if that functionality ever did land.
>> + 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.
Sounds good. Will fix.
Thanks for the feedback!
-john
Powered by blists - more mailing lists