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

Kees Cook
2011-05-25T18:01:28Z

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