All of
 help / color / mirror / Atom feed
From: John Stultz <>
To: lkml <>
Cc: John Stultz <>,
	Kees Cook <>,
	"Serge E. Hallyn" <>,
	Andrew Morton <>,
	Thomas Gleixner <>,
	Arjan van de Ven <>,
	Oren Laadan <>,
	Ruchi Kandoi <>,
	Rom Lemarchand <>, Todd Kjos <>,
	Colin Cross <>, Nick Kralevich <>,
	Dmitry Shmidt <>,
	Elliott Hughes <>,
	Android Kernel Team <>
Subject: Re: [PATCH 1/2 v4] proc: Relax /proc/<tid>/timerslack_ns capability requirements
Date: Mon, 1 Aug 2016 17:18:18 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Thu, Jul 21, 2016 at 1:24 PM, John Stultz <> 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


      parent reply	other threads:[~2016-08-02  0:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-21 20:24 John Stultz
2016-07-21 20:24 ` [PATCH 2/2 v4] proc: Add LSM hook checks to /proc/<tid>/timerslack_ns John Stultz
2016-08-17 21:21   ` Paul Moore
2016-08-17 21:36     ` John Stultz
2016-08-02  0:18 ` John Stultz [this message]

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:

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

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH 1/2 v4] proc: Relax /proc/<tid>/timerslack_ns capability requirements' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.