mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Nicolas Schichan <nschichan@freebox.fr>
Cc: Serge Hallyn <serge.hallyn@canonical.com>,
	Will Drewry <wad@chromium.org>, Kees Cook <keescook@chromium.org>,
	linux-kernel@vger.kernel.org, Eric Paris <eparis@redhat.com>,
	Mircea Gherzan <mgherzan@gmail.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	James Morris <james.l.morris@oracle.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH V2 1/3] seccomp: add generic code for jitted seccomp filters.
Date: Tue, 23 Apr 2013 16:43:03 -0700	[thread overview]
Message-ID: <20130423164303.d2eb663c4a1becea4a185087@linux-foundation.org> (raw)
In-Reply-To: <51752D98.8070709@freebox.fr>

On Mon, 22 Apr 2013 14:31:20 +0200 Nicolas Schichan <nschichan@freebox.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.

  reply	other threads:[~2013-04-23 23:43 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-18 14:50 [PATCH RFC] Support for JIT in Seccomp BPF filters Nicolas Schichan
2013-03-18 14:50 ` [PATCH V2 1/3] seccomp: add generic code for jitted seccomp filters Nicolas Schichan
2013-04-01 21:53   ` Kees Cook
2013-04-04 19:58     ` Will Drewry
2013-04-17 21:56   ` Andrew Morton
2013-04-22 12:31     ` Nicolas Schichan
2013-04-23 23:43       ` Andrew Morton [this message]
2013-04-24 15:52         ` Nicolas Schichan
2013-04-24 22:33           ` Andrew Morton
2013-03-18 14:50 ` [PATCH V2 2/3] ARM: net: bpf_jit: make code generation less dependent on struct sk_filter Nicolas Schichan
2013-04-05 12:01   ` Mircea Gherzan
2013-03-18 14:50 ` [PATCH V2 3/3] ARM: net: bpf_jit: add support for jitted seccomp filters Nicolas Schichan
2013-04-05 12:01   ` Mircea Gherzan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130423164303.d2eb663c4a1becea4a185087@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=eparis@redhat.com \
    --cc=james.l.morris@oracle.com \
    --cc=keescook@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgherzan@gmail.com \
    --cc=nschichan@freebox.fr \
    --cc=serge.hallyn@canonical.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=wad@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome