linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Stultz <john.stultz@linaro.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Kees Cook <keescook@chromium.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arjan van de Ven <arjan@linux.intel.com>,
	lkml <linux-kernel@vger.kernel.org>,
	Oren Laadan <orenl@cellrox.com>,
	Ruchi Kandoi <kandoiruchi@google.com>,
	Rom Lemarchand <romlem@android.com>,
	Android Kernel Team <kernel-team@android.com>
Subject: Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface
Date: Wed, 17 Feb 2016 14:29:29 -0800	[thread overview]
Message-ID: <CALAqxLVHBjPrfn8fK5W2ER7gkfMB2QoTthPAie9iQYaB-CdYfg@mail.gmail.com> (raw)
In-Reply-To: <20160217121813.8a49986ebf66cedaf43e901b@linux-foundation.org>

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.

thanks
-john

  parent reply	other threads:[~2016-02-17 22:29 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-17  1:06 [PATCH 0/2] Extend timer_slack_ns to u64 on 32bit systems & add /proc/<pid>/timerslack_ns John Stultz
2016-02-17  1:06 ` [PATCH 1/2] timer: Convert timer_slack_ns from unsigned long to u64 John Stultz
2016-02-17  1:06 ` [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface John Stultz
2016-02-17 19:35   ` Andrew Morton
2016-02-17 20:09     ` Kees Cook
2016-02-17 20:18       ` Andrew Morton
2016-02-17 20:51         ` John Stultz
2016-02-17 21:07           ` Andrew Morton
2016-02-17 22:29         ` John Stultz [this message]
2016-02-17 22:45           ` Kees Cook
2016-02-17 22:51             ` John Stultz
2016-02-17 22:53           ` Andrew Morton
2016-02-17 20:49     ` John Stultz
2016-02-18  5:59   ` [PATCH] proc: /proc/<pid>/timerslack_ns permissions fixes John Stultz
2016-02-18 17:52     ` Kees Cook
2016-07-13 23:47   ` [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface John Stultz
2016-07-14  3:39     ` Kees Cook
2016-07-14  5:29       ` Arjan van de Ven
2016-07-14 12:48       ` Serge E. Hallyn
2016-07-14 13:42         ` Arjan van de Ven
2016-07-14 16:01           ` John Stultz
2016-07-14 16:09         ` John Stultz
2016-07-14 17:45           ` Kees Cook
2016-07-14 17:48             ` Arjan van de Ven
2016-07-14 17:49             ` Serge E. Hallyn
2016-07-14 17:56               ` Kees Cook
2016-07-14 20:21                 ` Serge E. Hallyn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CALAqxLVHBjPrfn8fK5W2ER7gkfMB2QoTthPAie9iQYaB-CdYfg@mail.gmail.com \
    --to=john.stultz@linaro.org \
    --cc=akpm@linux-foundation.org \
    --cc=arjan@linux.intel.com \
    --cc=kandoiruchi@google.com \
    --cc=keescook@chromium.org \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=orenl@cellrox.com \
    --cc=romlem@android.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).