Messages in this thread | | | Date | Wed, 17 Feb 2016 14:45:40 -0800 | Subject | Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface | From | Kees Cook <> |
| |
On Wed, Feb 17, 2016 at 2:29 PM, John Stultz <john.stultz@linaro.org> wrote: > On Wed, Feb 17, 2016 at 12:18 PM, Andrew Morton > <akpm@linux-foundation.org> wrote: >> On Wed, 17 Feb 2016 12:09:08 -0800 Kees Cook <keescook@chromium.org> wrote: >>> On Wed, Feb 17, 2016 at 11:35 AM, Andrew Morton >>> > The procfs file's permissions are 0644, yes? So a process's >>> > timer_slack is world-readable? hm. >>> >>> This should be 600, IMO. >> >> Sounds safer. > > So I've gone ahead and addressed this and the other feedback you had. > But this bit made me realize that I may have missed a key aspect to > the interface that Android needs. > > In particular, the whole point here is to allow a controlling task to > modify other tasks' timerslack to limit background tasks' power usage > (and to modify them back to normal when the background tasks become > foreground tasks). Note that on android different tasks run as > different users. > > Currently, the controlling process has minimally elevated privileges > (CAP_SYS_NICE). The initial review suggested those privileges should > be higher (PTRACE_MODE_ATTACH), which I've implemented. However, I'm > realizing that by moving to the proc interface, the filesystem > permissions here put yet another barrier in the way. > > While the 600 permissions makes initial sense, it does limit these > controlling tasks with extra privileges (though not root) from > modifying the timerslack, since they cannot open the file to begin > with. > > So.... Does world writable (plus the PTRACE_MODE_ATTACH_FSCREDS check) > make more sense here? Or is there a better way for a system to tweak > the default permissions for procfs entries? (And if so, does that > render the PTRACE_MODE_ATTACH... check unnecessary?). > > Apologies. I'm fighting a head-cold, so I'm not feeling particularly sharp here.
Is timerslack sensitive at all? You could add the ptrace test to the _show function too, maybe. Then 0666 would solve the open issue without leaking the timerslack.
-Kees
-- Kees Cook Chrome OS & Brillo Security
| |