lkml.org 
[lkml]   [2009]   [Aug]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/4] perf_counter: Default to higher paranoia level

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, 2009-08-19 at 16:07 +0200, Peter Zijlstra wrote:
> > On Wed, 2009-08-19 at 11:18 +0200, Peter Zijlstra wrote:
> >
> > > +static inline bool perf_paranoid_anon(void)
> > > +{
> > > + return !capable(CAP_SYS_ADMIN) && sysctl_perf_counter_paranoid > 1;
> > > }
> > >
> > > static inline bool perf_paranoid_kernel(void)
> > > {
> > > - return sysctl_perf_counter_paranoid > 1;
> > > + return !capable(CAP_SYS_ADMIN) && sysctl_perf_counter_paranoid > 2;
> > > +}
> >
> > OK, this is buggy:
> >
> > - capable() uses current, which is unlikely to be counter->owner,
> > - but even security_real_capable(counter->owner, ...) wouldn't
> > work, since the ->capable() callback isn't NMI safe
> > (selinux takes locks and does allocations in that path).
> >
> > This puts a severe strain on more complex anonymizers since its
> > basically impossible to tell if counter->owner has permissions on
> > current from NMI context.
> >
> > I'll fix up this patch to pre-compute the perf_paranoid_anon_ip() per
> > counter based on creation time state, unless somebody has a better idea.
> >
> > I could possibly only anonymize IRQ context (SoftIRQ context is
> > difficult since in_softirq() means both in-softirq and
> > softirq-disabled).
>
> Something like the below maybe.. its 3 patches folded and I've no clue
> how to adapt the ppc code, what do people think?
>
> compile tested on x86-64 only.
>
> ---
> arch/x86/include/asm/stacktrace.h | 14 +++++--
> arch/x86/kernel/cpu/perf_counter.c | 66 +++++++++++++++++++++++++++++++----
> arch/x86/kernel/dumpstack.c | 10 +++--
> arch/x86/kernel/dumpstack_32.c | 8 +----
> arch/x86/kernel/dumpstack_64.c | 13 ++-----
> arch/x86/kernel/stacktrace.c | 8 +++--
> arch/x86/oprofile/backtrace.c | 5 ++-
> include/linux/perf_counter.h | 5 ++-
> kernel/perf_counter.c | 30 ++++++++++++----
> kernel/trace/trace_sysprof.c | 5 ++-
> 10 files changed, 117 insertions(+), 47 deletions(-)
>
> diff --git a/arch/x86/include/asm/stacktrace.h b/arch/x86/include/asm/stacktrace.h
> index cf86a5e..7066caa 100644
> --- a/arch/x86/include/asm/stacktrace.h
> +++ b/arch/x86/include/asm/stacktrace.h
> @@ -3,17 +3,23 @@
>
> extern int kstack_depth_to_print;
>
> -int x86_is_stack_id(int id, char *name);
> -
> /* Generic stack tracer with callbacks */
>
> +enum stack_type {
> + STACK_UNKNOWN = -1,
> + STACK_PROCESS = 0,
> + STACK_INTERRUPT = 1,
> + STACK_EXCEPTION = 2,
> +};
> +
> struct stacktrace_ops {
> void (*warning)(void *data, char *msg);
> /* msg must contain %s for the symbol */
> void (*warning_symbol)(void *data, char *msg, unsigned long symbol);
> - void (*address)(void *data, unsigned long address, int reliable);
> + void (*address)(void *data, unsigned long stack,
> + unsigned long address, int reliable);
> /* On negative return stop dumping */
> - int (*stack)(void *data, char *name);
> + int (*stack)(void *data, int type, char *name);
> };

nice generalization ...

> diff --git a/include/linux/perf_counter.h b/include/linux/perf_counter.h
> index 9ba1822..2b0528f 100644
> --- a/include/linux/perf_counter.h
> +++ b/include/linux/perf_counter.h
> @@ -439,6 +439,7 @@ enum perf_callchain_context {
> struct perf_callchain_entry {
> __u64 nr;
> __u64 ip[PERF_MAX_STACK_DEPTH];
> + int restricted;
> };

i'd love to have something more specific here - i.e. a context type
ID that identifies these basic types:

- process
- softirq
- hardirq
- NMI

and then let it be up to upper layers to decide what they do with a
restricted entry, and how to further process this information.

And it's not just security: for example it would be interesting to
sample pure, non-irq overhead - as IRQ overhead is often unrelated
to the process being measured.

Ingo


\
 
 \ /
  Last update: 2009-08-21 16:25    [W:0.099 / U:1.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site