On Thu, Jul 21, 2016 at 1:24 PM, John Stultz <[email protected]> wrote: > 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.
Ah, drat. I just realized that I missed changing from ptrace_may_access() to capable(CAP_SYS_NICE) means that a task cannot set its *own* timerslack value as is possible via the PR_SET_TIMERSLACK interface. Thus this patch, in trying to loosen the required privileges, actually adds a unnecessary restriction. I'm working on a patch that adds a check if p == current and allows the modification. Andrew: I know you queued this in -mm late, so I didn't think you'd send it to Linus yet, but in case you were considering it, please wait. thanks -john
