Re: [PATCH 3/5] v2 seccomp_filters: Enable ftrace-based system call filtering

[Kees Cook <kees.cook@canonical.com>]

Hi,

On Wed, May 25, 2011 at 07:48:51PM +0200, Thomas Gleixner wrote:
> On Wed, 25 May 2011, Ingo Molnar wrote:
> > * Thomas Gleixner <tglx@linutronix.de> wrote:
> > > On Tue, 24 May 2011, Ingo Molnar wrote:
> > > > * Peter Zijlstra <peterz@infradead.org> wrote:
> > > > 
> > > > > On Tue, 2011-05-24 at 10:59 -0500, Will Drewry wrote:
> > > > > >  include/linux/ftrace_event.h  |    4 +-
> > > > > >  include/linux/perf_event.h    |   10 +++++---
> > > > > >  kernel/perf_event.c           |   49 +++++++++++++++++++++++++++++++++++++---
> > > > > >  kernel/seccomp.c              |    8 ++++++
> > > > > >  kernel/trace/trace_syscalls.c |   27 +++++++++++++++++-----
> > > > > >  5 files changed, 82 insertions(+), 16 deletions(-) 
> > > > > 
> > > > > I strongly oppose to the perf core being mixed with any sekurity voodoo
> > > > > (or any other active role for that matter).
> > > > 
> > > > I'd object to invisible side-effects as well, and vehemently so. But note how 
> > > > intelligently it's used here: it's explicit in the code, it's used explicitly 
> > > > in kernel/seccomp.c and the event generation place in 
> > > > kernel/trace/trace_syscalls.c.
> > > > 
> > > > So this is a really flexible solution IMO and does not extend events with some 
> > > > invisible 'active' role. It extends the *call site* with an open-coded active 
> > > > role - which active role btw. already pre-existed.
> > > 
> > > We do _NOT_ make any decision based on the trace point so what's the
> > > "pre-existing" active role in the syscall entry code?
> > 
> > The seccomp code we are discussing in this thread.
> 
> That's proposed code and has absolutely nothing to do with the
> existing trace point semantics.
>  
> > > I'm all for code reuse and reuse of interfaces, but this is completely
> > > wrong. Instrumentation and security decisions are two fundamentally
> > > different things and we want them kept separate. Instrumentation is
> > > not meant to make decisions. Just because we can does not mean that it
> > > is a good idea.
> > 
> > Instrumentation does not 'make decisions': the calling site, which is 
> > already emitting both the event and wants to do decisions based on 
> > the data that also generates the event wants to do decisions.
> 
> You can repeat that as often as you want, it does not make it more
> true. Fact is that the decision is made in the middle of the perf code.

Can we just go back to the original spec? A lot of people were excited
about the prctl() API as done in Will's earlier patchset, we don't lose the
extremely useful "enable_on_exec" feature, and we can get away from all
this disagreement.

-Kees

-- 
Kees Cook
Ubuntu Security Team