linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Tightening up rdpmc permissions?
@ 2014-09-29 16:39 Andy Lutomirski
  2014-09-29 16:48 ` Peter Zijlstra
  2014-09-29 17:36 ` Valdis.Kletnieks
  0 siblings, 2 replies; 5+ messages in thread
From: Andy Lutomirski @ 2014-09-29 16:39 UTC (permalink / raw)
  To: Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	Arnaldo Carvalho de Melo, linux-kernel

I was surprised to notice that, by default, every task has permission
to use rdpmc.  seccomp cannot work around this.  This leaks
information, although the information leaked is of dubious and
variable value to an attacker.  It also renders the PR_TSC_SEGV
mechanism mostly useless.

Would it make sense to restrict rdpmc to tasks that are in mms that
have a perf_event mapping?  After all, unless I misunderstand
something, user code can't reliably use rdpmc unless they've mapped a
perf_event object to check the rdpmc bit and figure out what ecx value
to use.

I think that this could be implemented with very little overhead,
especially if combined with the existing CR4_TSD code and if that code
were taught to avoid reading cr4.

--Andy

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Tightening up rdpmc permissions?
  2014-09-29 16:39 Tightening up rdpmc permissions? Andy Lutomirski
@ 2014-09-29 16:48 ` Peter Zijlstra
  2014-09-29 17:36 ` Valdis.Kletnieks
  1 sibling, 0 replies; 5+ messages in thread
From: Peter Zijlstra @ 2014-09-29 16:48 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Paul Mackerras, Ingo Molnar, Arnaldo Carvalho de Melo, linux-kernel

On Mon, Sep 29, 2014 at 09:39:16AM -0700, Andy Lutomirski wrote:
> I was surprised to notice that, by default, every task has permission
> to use rdpmc. 

Right, we figured the paranoid would poke at
/sys/bus/event_source/devices/cpu/rdpmc.

> seccomp cannot work around this. 

I know nothing much about seccomp.

> This leaks information, although the information leaked is of dubious
> and variable value to an attacker.  It also renders the PR_TSC_SEGV
> mechanism mostly useless.

I'm not seeing how, there's no saying what will run on those counters,
let alone if its correlated to the TSC. But yes I appreciate the
argument.

> Would it make sense to restrict rdpmc to tasks that are in mms that
> have a perf_event mapping? 

We could,

> After all, unless I misunderstand
> something, user code can't reliably use rdpmc unless they've mapped a
> perf_event object to check the rdpmc bit and figure out what ecx value
> to use.

correct,

> I think that this could be implemented with very little overhead,
> especially if combined with the existing CR4_TSD code and if that code
> were taught to avoid reading cr4.

but there's a definite cost to having to write cr4 on every context
switch. It would be better if we could put it under TIF_NOTSC or
something similar and only be effective when something like seccomp or
other thingy is present and enabled. We should definitely avoid a double
cr4 write in case of both rdpmc and tsc switch.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Tightening up rdpmc permissions?
  2014-09-29 16:39 Tightening up rdpmc permissions? Andy Lutomirski
  2014-09-29 16:48 ` Peter Zijlstra
@ 2014-09-29 17:36 ` Valdis.Kletnieks
  2014-09-29 18:42   ` Andy Lutomirski
  1 sibling, 1 reply; 5+ messages in thread
From: Valdis.Kletnieks @ 2014-09-29 17:36 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	Arnaldo Carvalho de Melo, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 465 bytes --]

On Mon, 29 Sep 2014 09:39:16 -0700, Andy Lutomirski said:

> Would it make sense to restrict rdpmc to tasks that are in mms that
> have a perf_event mapping?  After all, unless I misunderstand
> something, user code can't reliably use rdpmc unless they've mapped a
> perf_event object to check the rdpmc bit and figure out what ecx value
> to use.

Wouldn't that be trivially easy for an attacker to bypass? Just map a dummy
perf_event object  and then go to town?

[-- Attachment #2: Type: application/pgp-signature, Size: 848 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Tightening up rdpmc permissions?
  2014-09-29 17:36 ` Valdis.Kletnieks
@ 2014-09-29 18:42   ` Andy Lutomirski
  2014-10-08 23:47     ` Andy Lutomirski
  0 siblings, 1 reply; 5+ messages in thread
From: Andy Lutomirski @ 2014-09-29 18:42 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: linux-kernel, Peter Zijlstra, Paul Mackerras,
	Arnaldo Carvalho de Melo, Ingo Molnar

On Sep 29, 2014 10:36 AM, <Valdis.Kletnieks@vt.edu> wrote:
>
> On Mon, 29 Sep 2014 09:39:16 -0700, Andy Lutomirski said:
>
> > Would it make sense to restrict rdpmc to tasks that are in mms that
> > have a perf_event mapping?  After all, unless I misunderstand
> > something, user code can't reliably use rdpmc unless they've mapped a
> > perf_event object to check the rdpmc bit and figure out what ecx value
> > to use.
>
> Wouldn't that be trivially easy for an attacker to bypass? Just map a dummy
> perf_event object  and then go to town?

Depends on the paranoia setting.  We could require that the mapped
object actually have an rdpmc-able counter running.

Seccomp can (and often does) block access to perf_event_open entirely.
We could certainly change the code to only twiddle CR4 if TIF_SECCOMP
or TIF_NOTSC is set.  I think that the real thing we should optimize
for is to minimize the chance that a given context switch actually
needs to *change* cr4.  Since perf_event maps are relatively unusual,
at least only perf-using programs would have overhead if we just gated
it on the existence of a useful rdpmc-able mapping.

(Also, why on earth is TIF_NOTSC a thread_info flag?  Wouldn't just
adding a field "cr4" to task_struct or something be simpler and quite
possibly faster?  We have a branch anyway...)

--Andy

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Tightening up rdpmc permissions?
  2014-09-29 18:42   ` Andy Lutomirski
@ 2014-10-08 23:47     ` Andy Lutomirski
  0 siblings, 0 replies; 5+ messages in thread
From: Andy Lutomirski @ 2014-10-08 23:47 UTC (permalink / raw)
  To: Valdis Kletnieks
  Cc: linux-kernel, Peter Zijlstra, Paul Mackerras,
	Arnaldo Carvalho de Melo, Ingo Molnar

On Mon, Sep 29, 2014 at 11:42 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Sep 29, 2014 10:36 AM, <Valdis.Kletnieks@vt.edu> wrote:
>>
>> On Mon, 29 Sep 2014 09:39:16 -0700, Andy Lutomirski said:
>>
>> > Would it make sense to restrict rdpmc to tasks that are in mms that
>> > have a perf_event mapping?  After all, unless I misunderstand
>> > something, user code can't reliably use rdpmc unless they've mapped a
>> > perf_event object to check the rdpmc bit and figure out what ecx value
>> > to use.
>>
>> Wouldn't that be trivially easy for an attacker to bypass? Just map a dummy
>> perf_event object  and then go to town?
>
> Depends on the paranoia setting.  We could require that the mapped
> object actually have an rdpmc-able counter running.
>
> Seccomp can (and often does) block access to perf_event_open entirely.
> We could certainly change the code to only twiddle CR4 if TIF_SECCOMP
> or TIF_NOTSC is set.  I think that the real thing we should optimize
> for is to minimize the chance that a given context switch actually
> needs to *change* cr4.  Since perf_event maps are relatively unusual,
> at least only perf-using programs would have overhead if we just gated
> it on the existence of a useful rdpmc-able mapping.
>
> (Also, why on earth is TIF_NOTSC a thread_info flag?  Wouldn't just
> adding a field "cr4" to task_struct or something be simpler and quite
> possibly faster?  We have a branch anyway...)

I have a prototype patch that seems to work and should have relatively
little overhead.  I'll send it either when the merge window closes or
when some pending dependencies get resolved.

--Andy

>
> --Andy



-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-10-08 23:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-29 16:39 Tightening up rdpmc permissions? Andy Lutomirski
2014-09-29 16:48 ` Peter Zijlstra
2014-09-29 17:36 ` Valdis.Kletnieks
2014-09-29 18:42   ` Andy Lutomirski
2014-10-08 23:47     ` Andy Lutomirski

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).