linux-kernel - Re: [PATCH 1/4] perf_counter: Default to higher paranoia level

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090821142151.GC11098@elte.hu>
Date:	Fri, 21 Aug 2009 16:21:51 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Paul Mackerras <paulus@...ba.org>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Mike Galbraith <efault@....de>, linux-kernel@...r.kernel.org,
	Jens Axboe <jens.axboe@...cle.com>,
	James Morris <jmorris@...ei.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 1/4] perf_counter: Default to higher paranoia level


* Peter Zijlstra <peterz@...radead.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/