Re: [PATCH 3/7] seccomp_filter: Enable ftrace-based system callfiltering

[Steven Rostedt]

On Thu, 2011-04-28 at 10:30 -0500, Will Drewry wrote:

> >> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> >> index 57d4b13..1bee87c 100644
> >> --- a/kernel/seccomp.c
> >> +++ b/kernel/seccomp.c
> >> @@ -8,10 +8,11 @@
> >>
> >>  #include <linux/seccomp.h>
> >>  #include <linux/sched.h>
> >> +#include <linux/slab.h>
> >>  #include <linux/compat.h>
> >>
> >>  /* #define SECCOMP_DEBUG 1 */
> >> -#define NR_SECCOMP_MODES 1
> >> +#define NR_SECCOMP_MODES 2
> >>
> >>  /*
> >>   * Secure computing mode 1 allows only read/write/exit/sigreturn.
> >> @@ -32,9 +33,11 @@ static int mode1_syscalls_32[] = {
> >>
> >>  void __secure_computing(int this_syscall)
> >>  {
> >> -     int mode = current->seccomp.mode;
> >> +     int mode = -1;
> >>       int * syscall;
> >> -
> >> +     /* Do we need an RCU read lock to access current's state? */
> >
> > I'm actually confused to why you are using RCU. What are you protecting.
> > Currently, I see the state is always accessed from current->seccomp. But
> > current should not be fighting with itself.
> >
> > Maybe I'm missing something.
> 
> I'm sure it's me that's missing something.  I believe the seccomp
> pointer can be accessed from:
> - current
> - via /proc/<pid>/seccomp_filter (read-only)
> 
> Given those cases, would it make sense to ditch the RCU interface for it?

Looking at this in a bit more detail. I think you can ditch the
rcu_readlocks where current accesses its own seccomp state. As current
is the one that duplicates it (and ups the refcount) on fork, and
current wont free it until after it performs a rcu_synchronization. No
one else can free current's seccomp state while current has a reference
to it.

You still need the rcu_readlocks on the readers for the proc system.
Otherwise the handle can be freed while its still in use. With the
rcu_readlocks, these readers will always get the refcount before current
frees it. And then the dec_and_test should work as expected when the
readers do the put.

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/