Quoting John Stultz (john.stu...@linaro.org): > When an interface to allow a task to change another tasks > timerslack was first proposed, it was suggested that something > greater then CAP_SYS_NICE would be needed, as a task could be > delayed further then what normally could be done with nice > adjustments. > > So CAP_SYS_PTRACE was adopted instead for what became the > /proc/<tid>/timerslack_ns interface. However, for Android (where > this feature originates), giving the system_server > CAP_SYS_PTRACE would allow it to observe and modify all tasks > memory. This is considered too high a privilege level for only > needing to change the timerslack. > > After some discussion, it was realized that a CAP_SYS_NICE > process can set a task as SCHED_FIFO, so they could fork some > spinning processes and set them all SCHED_FIFO 99, in effect > delaying all other tasks for an infinite amount of time. > > So as a CAP_SYS_NICE task can already cause trouble for other > tasks, using it as a required capability for accessing and > modifying /proc/<tid>/timerslack_ns seems sufficient. > > Thus, this patch loosens the capability requirements to > CAP_SYS_NICE and removes CAP_SYS_PTRACE, simplifying some > of the code flow as well. > > This is technically an ABI change, but as the feature just > landed in 4.6, I suspect no one is yet using it. > > Cc: Kees Cook <keesc...@chromium.org> > Cc: "Serge E. Hallyn" <se...@hallyn.com>
Acked-by: Serge Hallyn <se...@hallyn.com> > Cc: Andrew Morton <a...@linux-foundation.org> > Cc: Thomas Gleixner <t...@linutronix.de> > CC: Arjan van de Ven <ar...@linux.intel.com> > Cc: Oren Laadan <or...@cellrox.com> > Cc: Ruchi Kandoi <kandoiru...@google.com> > Cc: Rom Lemarchand <rom...@android.com> > Cc: Todd Kjos <tk...@google.com> > Cc: Colin Cross <ccr...@android.com> > Cc: Nick Kralevich <n...@google.com> > Cc: Dmitry Shmidt <dimitr...@google.com> > Cc: Elliott Hughes <e...@google.com> > Cc: Android Kernel Team <kernel-t...@android.com> > Signed-off-by: John Stultz <john.stu...@linaro.org> > --- > v2: Removed CAP_SYS_PTRACE check and simplified code flow > v3: Tweaked where CAP_SYS_NICE check is made, suggeded by NickK > > fs/proc/base.c | 34 ++++++++++++++++++++-------------- > 1 file changed, 20 insertions(+), 14 deletions(-) > > diff --git a/fs/proc/base.c b/fs/proc/base.c > index a11eb71..c94abae 100644 > --- a/fs/proc/base.c > +++ b/fs/proc/base.c > @@ -2281,16 +2281,19 @@ static ssize_t timerslack_ns_write(struct file *file, > const char __user *buf, > if (!p) > return -ESRCH; > > - if (ptrace_may_access(p, PTRACE_MODE_ATTACH_FSCREDS)) { > - task_lock(p); > - if (slack_ns == 0) > - p->timer_slack_ns = p->default_timer_slack_ns; > - else > - p->timer_slack_ns = slack_ns; > - task_unlock(p); > - } else > + if (!capable(CAP_SYS_NICE)) { > count = -EPERM; > + goto out; > + } > + > + task_lock(p); > + if (slack_ns == 0) > + p->timer_slack_ns = p->default_timer_slack_ns; > + else > + p->timer_slack_ns = slack_ns; > + task_unlock(p); > > +out: > put_task_struct(p); > > return count; > @@ -2300,19 +2303,22 @@ static int timerslack_ns_show(struct seq_file *m, > void *v) > { > struct inode *inode = m->private; > struct task_struct *p; > - int err = 0; > + int err = 0; > > p = get_proc_task(inode); > if (!p) > return -ESRCH; > > - if (ptrace_may_access(p, PTRACE_MODE_ATTACH_FSCREDS)) { > - task_lock(p); > - seq_printf(m, "%llu\n", p->timer_slack_ns); > - task_unlock(p); > - } else > + if (!capable(CAP_SYS_NICE)) { > err = -EPERM; > + goto out; > + } > > + task_lock(p); > + seq_printf(m, "%llu\n", p->timer_slack_ns); > + task_unlock(p); > + > +out: > put_task_struct(p); > > return err; > -- > 1.9.1