Date:	Wed, 4 May 2011 20:30:55 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Will Drewry <wad@...omium.org>, Eric Paris <eparis@...hat.com>,
	Ingo Molnar <mingo@...e.hu>, linux-kernel@...r.kernel.org,
	kees.cook@...onical.com, agl@...omium.org, jmorris@...ei.org,
	Randy Dunlap <rdunlap@...otime.net>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Tom Zanussi <tzanussi@...il.com>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 5/7] seccomp_filter: Document what seccomp_filter is
 and how it works.

On Wed, May 04, 2011 at 02:23:02PM -0400, Steven Rostedt wrote:
> On Wed, 2011-05-04 at 19:52 +0200, Frederic Weisbecker wrote:
> 
> > >  It's certainly doable, but it will
> > > mean that we may be logically storing something like:
> > > 
> > > __NR_foo: (a == 1 || a == 2), applied
> > > __NR_foo: b == 2, not applied
> > > __NR_foo: c == 3, not applied
> > > 
> > > after
> > > 
> > > SECCOMP_FILTER_SET, __NR_foo, "a == 1 || a == 2"
> > > SECCOMP_FILTER_APPLY
> > > SECCOMP_FILTER_SET, __NR_foo, "b == 2"
> > > SECCOMP_FILTER_SET, __NR_foo, "c == 3"
> > 
> > No, the c == 3 would override b == 2.
> 
> I honestly hate the "override" mode. I like that SETs are or'd among
> each other and an APPLY is a "commit"; meaning that you can only limit
> it further, but can not extend it (an explicit &&)

I'm confused with what you just said...

Somehow I could understand it that way:

SECCOMP_FILTER_SET, __NR_foo, "a == 1"
SECCOMP_FILTER_SET, __NR_foo, "a == 2"
SECCOMP_FILTER_APPLY
SECCOMP_FILTER_SET, __NR_foo, "b == 1"

Would produce:

"(a == 1 || a == 2) && b == 1"

That makes a pretty confusing behaviour for users I think.

> 
> > 
> > > In that case, would a call to sys_foo even be tested against the
> > > non-applied constraints of b==2 or c==3?
> > 
> > No, not as long as it's not applied.
> > 
> > > Or would the call to set "c
> > > == 3" replace the "b == 2" entry.  I'm not sure I see that the benefit
> > > exceeds the ambiguity that might introduce.
> > 
> > The rationale behind it is that as long as you haven't applied your filter,
> > you should be able to override it.
> 
> We need a "UNSET" (I like that better than DROP).

What about a complete erase (RESET) of the temporary filter? Like I explained below
from my previous mail.

> 
> > 
> > And the simplest way to override it is to do it completely. Remove what
> > was there before (that wasn't applied).
> > 
> > You could opt for a FILTER_DROP.
> > Let's take that example:
> > 
> > SECCOMP_FILTER_SET, __NR_foo, "b == 2"
> > SECCOMP_FILTER_SET, __NR_foo, "c == 3"
> > 
> > Following your logic, we should have "b == 2 && c == 3" after that.
> > How are you going to remove only the c == 3 sequence?
> > 
> > You would need SECCOMP_FILTER_DROP, __NR_foo, "c == 3"
> > 
> > That said, using a string with a filter expression as an id looks a
> > bit odd to me. That doesn't work anymore with "((c == 3))", or "c== 3",
> > unless you compare against the resulting tree but that's complicated.
> 
> Nah, it should be easy. Actually, in "set" mode (before the apply) we
> should keep a link list of "trees" of sets. Then we just find the "tree"
> that matches the UNSET and remove it.
> 
> > 
> > I mean, that works, most of the time you keep your expression
> > and pass it as is. But the expression should be identified by its
> > meaning, not by some random language layout.
> > 
> > That also forces you to scatter your filter representation:
> > 
> > 	SECCOMP_FILTER_SET, __NR_foo, "b == 2"
> > 	SECCOMP_FILTER_SET, __NR_foo, "c == 3"
> > 
> > For me this shouldn't be different than
> > 
> > 	SECCOMP_FILTER_SET, __NR_foo, "b == 2 && c == 3"
> > 
> > Still nobody will be able to remove a part of the above expression.
> 
> Correct, as the sets will be just link lists of sets. Once we apply it,
> it goes into the main tree.
> 
> Thus, to find the UNSET set, we would evaluate the tree of that unset,
> and search for it. If it is found, remove it, otherwise return EINVAL or
> something.
> 
> > 
> > So, I'm more for having something that removes everything not applied
> > than just a part of the non-applied filter.
> > 
> > Two possibilities I see:
> > 
> > 	SECCOMP_FILTER_SET, __NR_foo, "b == 2"
> > 	SECCOMP_FILTER_SET, __NR_foo, "c == 3"
> > 
> > 	// And result is "c == 3"
> > 
> > Or:
> > 
> > 	SECCOMP_FILTER_SET, __NR_foo, "b == 2"
> > 	SECCOMP_FILTER_SET, __NR_foo, "c == 3"
> > 
> > 	// And result is "b == 2 && c == 3"
> > 
> > 	SECCOMP_FILTER_RESET, __NR_foo //rewind to filter "0"
> > 
> > 	SECCOMP_FILTER_SET, __NR_foo, "c == 4"
> > 
> > 	// And result is "c == 4"
> > 
> > How does that look?
> 
> The thing is, I don't understand where we would use (or want) an
> override. I could understand a UNSET, which I described in another
> reply. Also, I like the fact that sets can be self contained because
> then the user can add wrapper functions for them.
> 
> add_filter("foo: c == 4");
> add_filter("bar: b < 3");
> apply_filter();
> 
> That wont work with an override, unless we did the work in userspace to
> keep string, and then we need to worry about "string matches" as you
> stated if we want to implement a remove_filter("foo: c==4"). (see it
> would fail, because this version is missing spaces)
> 
> -- Steve
> 
> 
--
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/

Powered by blists - more mailing lists