Messages in this thread | | | Date | Thu, 12 Jan 2012 15:50:40 +0100 | From | Oleg Nesterov <> | Subject | Re: [RFC,PATCH 1/2] seccomp_filters: system call filtering using BPF |
| |
On 01/11, Will Drewry wrote: > > This patch adds support for seccomp mode 2. This mode enables dynamic > enforcement of system call filtering policy in the kernel as specified > by a userland task. The policy is expressed in terms of a BPF program, > as is used for userland-exposed socket filtering. Instead of network > data, the BPF program is evaluated over struct user_regs_struct at the > time of the system call (as retrieved using regviews).
Cool ;)
I didn't really read this patch yet, just one nit.
> +#define seccomp_filter_init_task(_tsk) do { \ > + (_tsk)->seccomp.filter = NULL; \ > +} while (0);
Cosmetic and subjective, but imho it would be better to add inline functions instead of define's.
> @@ -166,6 +167,7 @@ void free_task(struct task_struct *tsk) > free_thread_info(tsk->stack); > rt_mutex_debug_task_free(tsk); > ftrace_graph_exit_task(tsk); > + seccomp_filter_free_task(tsk); > free_task_struct(tsk); > } > EXPORT_SYMBOL(free_task); > @@ -1209,6 +1211,7 @@ static struct task_struct *copy_process(unsigned long clone_flags, > /* Perform scheduler related setup. Assign this task to a CPU. */ > sched_fork(p); > > + seccomp_filter_init_task(p);
This doesn't look right or I missed something. something seccomp_filter_init_task() should be called right after dup_task_struct(), at least before copy process can fail.
Otherwise copy_process()->free_fork()->seccomp_filter_free_task() can put current->seccomp.filter copied by arch_dup_task_struct().
> +struct seccomp_filter { > + struct kref usage; > + struct pid *creator;
Why? seccomp_filter->creator is never used, no?
Oleg.
| |