linux-kernel - Re: [PATCH V2 1/3] seccomp: add generic code for jitted seccomp filters.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 23 Apr 2013 16:43:03 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Nicolas Schichan <nschichan@...ebox.fr>
Cc:	Serge Hallyn <serge.hallyn@...onical.com>,
	Will Drewry <wad@...omium.org>,
	Kees Cook <keescook@...omium.org>,
	linux-kernel@...r.kernel.org, Eric Paris <eparis@...hat.com>,
	Mircea Gherzan <mgherzan@...il.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	James Morris <james.l.morris@...cle.com>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH V2 1/3] seccomp: add generic code for jitted seccomp
 filters.

On Mon, 22 Apr 2013 14:31:20 +0200 Nicolas Schichan <nschichan@...ebox.fr> wrote:

> On 04/17/2013 11:56 PM, Andrew Morton wrote:
> > This patch is killing me.
> >
> >> --- a/include/linux/seccomp.h
> >> +++ b/include/linux/seccomp.h
> >> @@ -6,6 +6,7 @@
> >>   #ifdef CONFIG_SECCOMP
> >>
> >>   #include <linux/thread_info.h>
> >> +#include <linux/filter.h>
> >>   #include <asm/seccomp.h>
> >
> > In file included from include/linux/compat.h:18,
> >                   from include/linux/filter.h:9,
> >                   from include/linux/seccomp.h:9,
> >                   from include/linux/sched.h:39,
> >                   from arch/x86/kernel/asm-offsets.c:9:
> > /usr/src/25/arch/x86/include/asm/compat.h: In function 'arch_compat_alloc_user_space':
> > /usr/src/25/arch/x86/include/asm/compat.h:301: error: dereferencing pointer to incomplete type
> >
> > Problem is, compat.h's arch_compat_alloc_user_space() needs sched.h for
> > task_struct but as you can see from the above include tree, sched.h
> > includes seccomp.h and everything falls over.  The preprocessed code
> > contains the definition of arch_compat_alloc_user_space() *before* the
> > definition of task_struct.
> >
> > This is a basic x86_64 "make clean; make allmodconfig; make".
> 
> Hi,
> 
> Would including <uapi/linux/filter.h> instead of <linux/filter.h> in seccomp.h 
> be an acceptable solution ?
> 
> I have tried that and (with an additional forward declaration of struct 
> sk_buff) an x86_64 "make clean; make allmodconfig" run finishes succesfully.
> 
> If that's ok with you, I can resend the serie with that fix.

It would be better to make the code and include tangle less complex,
rather than more complex.

Did we really need to move the `struct seccomp_filter' definition into
the header file?  afaict that wasn't really necessary - we can add a
few helper functions to kernel/seccomp.c and then have the remote code
treat seccomp_filter in an opaque fashion rather than directly poking
at its internals.


btw, what on earth is going on with seccomp_jit_free()?  It does
disturbing undocumented typecasting and it punts the module_free into a
kernel thread for mysterious, undocumented and possibly buggy reasons.

I realize it just copies bpf_jit_free().  The same observations apply there.
--
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/