Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1442946 > unrolled thread

Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface John Stultz <john.stultz@linaro.org> - 2016-07-14 01:50 +0200
    Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface Kees Cook <keescook@chromium.org> - 2016-07-14 05:40 +0200
      Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface Arjan van de Ven <arjan@linux.intel.com> - 2016-07-14 07:30 +0200
      Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface "Serge E. Hallyn" <serge@hallyn.com> - 2016-07-14 14:50 +0200
        Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface Arjan van de Ven <arjan@linux.intel.com> - 2016-07-14 15:50 +0200
          Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface John Stultz <john.stultz@linaro.org> - 2016-07-14 18:10 +0200
        Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface John Stultz <john.stultz@linaro.org> - 2016-07-14 18:10 +0200
          Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface Kees Cook <keescook@chromium.org> - 2016-07-14 19:50 +0200
            Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface Arjan van de Ven <arjan@linux.intel.com> - 2016-07-14 19:50 +0200
            Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface "Serge E. Hallyn" <serge@hallyn.com> - 2016-07-14 19:50 +0200
              Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface Kees Cook <keescook@chromium.org> - 2016-07-14 20:00 +0200
                Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface "Serge E. Hallyn" <serge@hallyn.com> - 2016-07-14 22:30 +0200

#1442946 — Re: [PATCH 2/2] proc: Add /proc/<pid>/timerslack_ns interface

On Tue, Feb 16, 2016 at 5:06 PM, John Stultz <john.stultz@linaro.org> wrote:
> This patch provides a proc/PID/timerslack_ns interface which
> exposes a task's timerslack value in nanoseconds and allows it
> to be changed.
>
> This allows power/performance management software to set timer
> slack for other threads according to its policy for the thread
> (such as when the thread is designated foreground vs. background
> activity)
>
> If the value written is non-zero, slack is set to that value.
> Otherwise sets it to the default for the thread.
>
> This interface checks that the calling task has permissions to
> to use PTRACE_MODE_ATTACH_FSCREDS on the target task, so that we
> can ensure arbitrary apps do not change the timer slack for other
> apps.

Sigh.

So I wanted to pull this thread up again, because when I originally
proposed upstreaming the PR_SET_TIMERSLACK_PID feature from the AOSP
common.git tree, the first objection from Arjan was that it only
required CAP_SYS_NICE:
   http://lkml.iu.edu/hypermail/linux/kernel/1506.3/01491.html

And reasonably, setting timerslack to very large values does have the
potential to effect applications much further then what a task could
do previously with CAP_SYS_NICE.

CAP_SYS_PTRACE was suggested instead, as that allows applications to
manipulate other tasks more drastically.

(At the time, I checked with some of the Android developers, and got
no objection to changing to use this capability.)

However, after submitting the changes to Android required to support
the upstreamed /proc/<tid>/timerslack_ns interface, I've gotten some
objections with adding CAP_SYS_PTRACE to the system_server, as this
would allow the system_server to be able to inspect and modify memory
on any task in the system. This gives the system_server privileged to
effect applications much further then what it could do previously.

So I worry I'm a bit stuck here. For general systems, CAP_SYS_NICE is
too low a level of privilege  to set a tasks timerslack, but
apparently CAP_SYS_PTRACE is too high a privilege for Android's
system_server to require just to set a tasks timerslack value.

So I wanted to ask again if we might consider backing this down to
CAP_SYS_NICE, or if we can instead introduce a new CAP_SYS_TIMERSLACK
or something to provide the needed in-between capability level.

Thoughts?

thanks
-john

[toc] | [next] | [standalone]

#1443049

On Wed, Jul 13, 2016 at 4:47 PM, John Stultz <john.stultz@linaro.org> wrote:
> On Tue, Feb 16, 2016 at 5:06 PM, John Stultz <john.stultz@linaro.org> wrote:
>> This patch provides a proc/PID/timerslack_ns interface which
>> exposes a task's timerslack value in nanoseconds and allows it
>> to be changed.
>>
>> This allows power/performance management software to set timer
>> slack for other threads according to its policy for the thread
>> (such as when the thread is designated foreground vs. background
>> activity)
>>
>> If the value written is non-zero, slack is set to that value.
>> Otherwise sets it to the default for the thread.
>>
>> This interface checks that the calling task has permissions to
>> to use PTRACE_MODE_ATTACH_FSCREDS on the target task, so that we
>> can ensure arbitrary apps do not change the timer slack for other
>> apps.
>
> Sigh.
>
> So I wanted to pull this thread up again, because when I originally
> proposed upstreaming the PR_SET_TIMERSLACK_PID feature from the AOSP
> common.git tree, the first objection from Arjan was that it only
> required CAP_SYS_NICE:
>    http://lkml.iu.edu/hypermail/linux/kernel/1506.3/01491.html
>
> And reasonably, setting timerslack to very large values does have the
> potential to effect applications much further then what a task could
> do previously with CAP_SYS_NICE.
>
> CAP_SYS_PTRACE was suggested instead, as that allows applications to
> manipulate other tasks more drastically.
>
> (At the time, I checked with some of the Android developers, and got
> no objection to changing to use this capability.)
>
> However, after submitting the changes to Android required to support
> the upstreamed /proc/<tid>/timerslack_ns interface, I've gotten some
> objections with adding CAP_SYS_PTRACE to the system_server, as this
> would allow the system_server to be able to inspect and modify memory
> on any task in the system. This gives the system_server privileged to
> effect applications much further then what it could do previously.
>
> So I worry I'm a bit stuck here. For general systems, CAP_SYS_NICE is
> too low a level of privilege  to set a tasks timerslack, but
> apparently CAP_SYS_PTRACE is too high a privilege for Android's
> system_server to require just to set a tasks timerslack value.
>
> So I wanted to ask again if we might consider backing this down to
> CAP_SYS_NICE, or if we can instead introduce a new CAP_SYS_TIMERSLACK
> or something to provide the needed in-between capability level.

Adding new capabilities appears to not really be viable (lots of
threads about this...)

I think the original CAP_SYS_NICE should be fine. A malicious
CAP_SYS_NICE process can do plenty of insane things, I don't feel like
the timer slack adds to any realistic risks.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1443072

On 7/13/2016 8:39 PM, Kees Cook wrote:
>>
>> So I worry I'm a bit stuck here. For general systems, CAP_SYS_NICE is
>> too low a level of privilege  to set a tasks timerslack, but
>> apparently CAP_SYS_PTRACE is too high a privilege for Android's
>> system_server to require just to set a tasks timerslack value.
>>
>> So I wanted to ask again if we might consider backing this down to
>> CAP_SYS_NICE, or if we can instead introduce a new CAP_SYS_TIMERSLACK
>> or something to provide the needed in-between capability level.
>
> Adding new capabilities appears to not really be viable (lots of
> threads about this...)
>
> I think the original CAP_SYS_NICE should be fine. A malicious
> CAP_SYS_NICE process can do plenty of insane things, I don't feel like
> the timer slack adds to any realistic risks.

if the result is really as bad as you describe, then that is worse than
the impact of this being CAP_SYS_NICE, and thus SYS_TRACE is maybe the
purist answer, but not the pragmatic best answer; certainly I don't want
to make the overall system security worse.

I wonder how much you want to set the slack; one of the options (and I don't
know how this will work in the code, if it's horrible don't do it)
is to limit how much slack CAP_SYS_NICE can set (say, 50 or 100 msec, e.g. in the order
of a "time slice" or two if Linux had time slices, similar to what nice would do)
while CAP_SYS_TRACE  can set the full 4 seconds.
If it makes the code horrible, don't do it and just do SYS_NICE.

[toc] | [prev] | [next] | [standalone]

#1443422

Quoting Kees Cook (keescook@chromium.org):
> On Wed, Jul 13, 2016 at 4:47 PM, John Stultz <john.stultz@linaro.org> wrote:
> > On Tue, Feb 16, 2016 at 5:06 PM, John Stultz <john.stultz@linaro.org> wrote:
> >> This patch provides a proc/PID/timerslack_ns interface which
> >> exposes a task's timerslack value in nanoseconds and allows it
> >> to be changed.
> >>
> >> This allows power/performance management software to set timer
> >> slack for other threads according to its policy for the thread
> >> (such as when the thread is designated foreground vs. background
> >> activity)
> >>
> >> If the value written is non-zero, slack is set to that value.
> >> Otherwise sets it to the default for the thread.
> >>
> >> This interface checks that the calling task has permissions to
> >> to use PTRACE_MODE_ATTACH_FSCREDS on the target task, so that we
> >> can ensure arbitrary apps do not change the timer slack for other
> >> apps.
> >
> > Sigh.
> >
> > So I wanted to pull this thread up again, because when I originally
> > proposed upstreaming the PR_SET_TIMERSLACK_PID feature from the AOSP
> > common.git tree, the first objection from Arjan was that it only
> > required CAP_SYS_NICE:
> >    http://lkml.iu.edu/hypermail/linux/kernel/1506.3/01491.html
> >
> > And reasonably, setting timerslack to very large values does have the
> > potential to effect applications much further then what a task could
> > do previously with CAP_SYS_NICE.
> >
> > CAP_SYS_PTRACE was suggested instead, as that allows applications to
> > manipulate other tasks more drastically.
> >
> > (At the time, I checked with some of the Android developers, and got
> > no objection to changing to use this capability.)
> >
> > However, after submitting the changes to Android required to support
> > the upstreamed /proc/<tid>/timerslack_ns interface, I've gotten some
> > objections with adding CAP_SYS_PTRACE to the system_server, as this
> > would allow the system_server to be able to inspect and modify memory
> > on any task in the system. This gives the system_server privileged to
> > effect applications much further then what it could do previously.
> >
> > So I worry I'm a bit stuck here. For general systems, CAP_SYS_NICE is
> > too low a level of privilege  to set a tasks timerslack, but
> > apparently CAP_SYS_PTRACE is too high a privilege for Android's
> > system_server to require just to set a tasks timerslack value.
> >
> > So I wanted to ask again if we might consider backing this down to
> > CAP_SYS_NICE, or if we can instead introduce a new CAP_SYS_TIMERSLACK
> > or something to provide the needed in-between capability level.
> 
> Adding new capabilities appears to not really be viable (lots of
> threads about this...)

Sorry - why is this?

> I think the original CAP_SYS_NICE should be fine. A malicious
> CAP_SYS_NICE process can do plenty of insane things, I don't feel like
> the timer slack adds to any realistic risks.

Can someone give a detailed explanation of what you could do with
the new timerslack feature and compare it to what you can do with
sys_nice?

[toc] | [prev] | [next] | [standalone]

#1443469

On 7/14/2016 5:48 AM, Serge E. Hallyn wrote:

> Can someone give a detailed explanation of what you could do with
> the new timerslack feature and compare it to what you can do with
> sys_nice?
>

what you can do with the timerslack feature is add upto 4 seconds of extra
time/delay on top of each select()/poll()/nanosleep()/... (basically anything that
uses hrtimers on behalf of the user), and then also control within that
4 second window exactly when that extra delay ends
(which may help a timing attack kind of scenario)

[toc] | [prev] | [next] | [standalone]

#1443564

On Thu, Jul 14, 2016 at 6:42 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
> On 7/14/2016 5:48 AM, Serge E. Hallyn wrote:
>
>> Can someone give a detailed explanation of what you could do with
>> the new timerslack feature and compare it to what you can do with
>> sys_nice?
>>
>
> what you can do with the timerslack feature is add upto 4 seconds of extra
> time/delay on top of each select()/poll()/nanosleep()/... (basically
> anything that
> uses hrtimers on behalf of the user), and then also control within that
> 4 second window exactly when that extra delay ends
> (which may help a timing attack kind of scenario)

So the interface actually allows for 64bits of nanoseconds, so more or
less infinite delay if nothing else is happening.

thanks
-john

[toc] | [prev] | [next] | [standalone]

#1443557

On Thu, Jul 14, 2016 at 5:48 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> Quoting Kees Cook (keescook@chromium.org):
>> I think the original CAP_SYS_NICE should be fine. A malicious
>> CAP_SYS_NICE process can do plenty of insane things, I don't feel like
>> the timer slack adds to any realistic risks.
>
> Can someone give a detailed explanation of what you could do with
> the new timerslack feature and compare it to what you can do with
> sys_nice?

Looking at the man page for CAP_SYS_NICE, it looks like such a task
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 one might argue setting large timerslack vlaues isn't that
different risk wise?

thanks
-john

[toc] | [prev] | [next] | [standalone]

#1443637

On Thu, Jul 14, 2016 at 9:09 AM, John Stultz <john.stultz@linaro.org> wrote:
> On Thu, Jul 14, 2016 at 5:48 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
>> Quoting Kees Cook (keescook@chromium.org):
>>> I think the original CAP_SYS_NICE should be fine. A malicious
>>> CAP_SYS_NICE process can do plenty of insane things, I don't feel like
>>> the timer slack adds to any realistic risks.
>>
>> Can someone give a detailed explanation of what you could do with
>> the new timerslack feature and compare it to what you can do with
>> sys_nice?
>
> Looking at the man page for CAP_SYS_NICE, it looks like such a task
> 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 one might argue setting large timerslack vlaues isn't that
> different risk wise?

Right -- you can hose a system with CAP_SYS_NICE already; I don't
think timerslack realistically changes that.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1443640

On 7/14/2016 10:45 AM, Kees Cook wrote:
> On Thu, Jul 14, 2016 at 9:09 AM, John Stultz <john.stultz@linaro.org> wrote:
>> On Thu, Jul 14, 2016 at 5:48 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
>>> Quoting Kees Cook (keescook@chromium.org):
>>>> I think the original CAP_SYS_NICE should be fine. A malicious
>>>> CAP_SYS_NICE process can do plenty of insane things, I don't feel like
>>>> the timer slack adds to any realistic risks.
>>>
>>> Can someone give a detailed explanation of what you could do with
>>> the new timerslack feature and compare it to what you can do with
>>> sys_nice?
>>
>> Looking at the man page for CAP_SYS_NICE, it looks like such a task
>> 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 one might argue setting large timerslack vlaues isn't that
>> different risk wise?
>
> Right -- you can hose a system with CAP_SYS_NICE already; I don't
> think timerslack realistically changes that.

fair enough

the worry of being able to time attack things is there already with the SCHED_FIFO
so... purist objection withdrawn in favor of the pragmatic

[toc] | [prev] | [next] | [standalone]

#1443643

Quoting Kees Cook (keescook@chromium.org):
> On Thu, Jul 14, 2016 at 9:09 AM, John Stultz <john.stultz@linaro.org> wrote:
> > On Thu, Jul 14, 2016 at 5:48 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> >> Quoting Kees Cook (keescook@chromium.org):
> >>> I think the original CAP_SYS_NICE should be fine. A malicious
> >>> CAP_SYS_NICE process can do plenty of insane things, I don't feel like
> >>> the timer slack adds to any realistic risks.
> >>
> >> Can someone give a detailed explanation of what you could do with
> >> the new timerslack feature and compare it to what you can do with
> >> sys_nice?
> >
> > Looking at the man page for CAP_SYS_NICE, it looks like such a task
> > 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 one might argue setting large timerslack vlaues isn't that
> > different risk wise?
> 
> Right -- you can hose a system with CAP_SYS_NICE already; I don't
> think timerslack realistically changes that.

Thanks - so it seems to me if we go with CAP_SYS_NICE we are giving
those who can already hose the system another vector to doing so.  But
if we require CAP_SYS_PTRACE then we are giving those who can newly hose
the system also the ability to subvert any task.  It sounds like
CAP_SYS_NICE is the winner.

Kees, you said adding a capability is hard - can you expound on that?

[toc] | [prev] | [next] | [standalone]

#1443647

On Thu, Jul 14, 2016 at 10:49 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> Kees, you said adding a capability is hard - can you expound on that?

Best I can find at the moment was discussion around CAP_COMPROMISE_KERNEL:
http://thread.gmane.org/gmane.linux.kernel/1459165

Basically, adding a new capability for an interface can create
userspace compatibility problems (though perhaps in this case, it's a
new interface, so a new capability would be okay, but it's such a
narrow use-case and CAP_SYS_NICE fits fine).

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1443745

Quoting Kees Cook (keescook@chromium.org):
> On Thu, Jul 14, 2016 at 10:49 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> > Kees, you said adding a capability is hard - can you expound on that?
> 
> Best I can find at the moment was discussion around CAP_COMPROMISE_KERNEL:
> http://thread.gmane.org/gmane.linux.kernel/1459165

Hm, the last discussion I recall around that topic involved a confusing
negative capability iirc, I assume CAP_COMPROMISE_KERNEL was the revamped
version.

> Basically, adding a new capability for an interface can create
> userspace compatibility problems (though perhaps in this case, it's a
> new interface, so a new capability would be okay, but it's such a
> narrow use-case and CAP_SYS_NICE fits fine).

Right, there are two ways they can be added.  For new functionality,
no big deal.  (Of course we'd like to avoid going beyond 64 bits of cap
too soon, so don't want to go crazy).

The other is when we want to split off a more fine-grained version of
an existing capability.  Then we just have to make sure that the coarser
pre-existing capability continues to work as expected.  Breaking out
CAP_SYSLOG from CAP_SYS_ADMIN was an example of that.

-serge

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web