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: Wed, 24 Apr 2013 15:33:23 -0700 [thread overview]
Message-ID: <20130424153323.73901c8e026580dc279329b0@linux-foundation.org> (raw)
In-Reply-To: <5177FFC2.5020102@freebox.fr>
On Wed, 24 Apr 2013 17:52:34 +0200 Nicolas Schichan <nschichan@freebox.fr> wrote:
> > 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.
>
> The reason for this hack for both seccomp filters and socket filters is that
> {seccomp,bpf}_jit_free are called from a softirq. module_free() cannot be
> called directly from softirq, as it will in turn call vfree() which will
> BUG_ON() if in_interrupt() is non zero. So to call module_free(), it is
> therefore required to be in a process context, which is provided by the work
> struct.
Well let's explain this to the next sucker who comes along.
From: Andrew Morton <akpm@linux-foundation.org>
Subject: bpf: add comments explaining the schedule_work() operation
Cc: Will Drewry <wad@chromium.org>
Cc: Mircea Gherzan <mgherzan@gmail.com>
Cc: Nicolas Schichan <nschichan@freebox.fr>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Daniel Borkmann <daniel.borkmann@tik.ee.ethz.ch>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
arch/arm/net/bpf_jit_32.c | 8 ++++++++
arch/powerpc/net/bpf_jit_comp.c | 4 ++++
arch/s390/net/bpf_jit_comp.c | 4 ++++
arch/sparc/net/bpf_jit_comp.c | 4 ++++
arch/x86/net/bpf_jit_comp.c | 4 ++++
5 files changed, 24 insertions(+)
diff -puN arch/arm/net/bpf_jit_32.c~a arch/arm/net/bpf_jit_32.c
--- a/arch/arm/net/bpf_jit_32.c~a
+++ a/arch/arm/net/bpf_jit_32.c
@@ -958,6 +958,10 @@ void bpf_jit_free(struct sk_filter *fp)
struct work_struct *work;
if (fp->bpf_func != sk_run_filter) {
+ /*
+ * bpf_jit_free() can be called from softirq; module_free()
+ * requires process context.
+ */
work = (struct work_struct *)fp->bpf_func;
INIT_WORK(work, bpf_jit_free_worker);
@@ -985,6 +989,10 @@ void seccomp_jit_free(struct seccomp_fil
void *bpf_func = seccomp_filter_get_bpf_func(fp);
if (bpf_func != sk_run_filter) {
+ /*
+ * seccomp_jit_free() can be called from softirq; module_free()
+ * requires process context.
+ */
work = (struct work_struct *)bpf_func;
INIT_WORK(work, bpf_jit_free_worker);
diff -puN arch/sparc/net/bpf_jit_comp.c~a arch/sparc/net/bpf_jit_comp.c
--- a/arch/sparc/net/bpf_jit_comp.c~a
+++ a/arch/sparc/net/bpf_jit_comp.c
@@ -817,6 +817,10 @@ static void jit_free_defer(struct work_s
void bpf_jit_free(struct sk_filter *fp)
{
if (fp->bpf_func != sk_run_filter) {
+ /*
+ * bpf_jit_free() can be called from softirq; module_free()
+ * requires process context.
+ */
struct work_struct *work = (struct work_struct *)fp->bpf_func;
INIT_WORK(work, jit_free_defer);
diff -puN arch/powerpc/net/bpf_jit_comp.c~a arch/powerpc/net/bpf_jit_comp.c
--- a/arch/powerpc/net/bpf_jit_comp.c~a
+++ a/arch/powerpc/net/bpf_jit_comp.c
@@ -699,6 +699,10 @@ static void jit_free_defer(struct work_s
void bpf_jit_free(struct sk_filter *fp)
{
if (fp->bpf_func != sk_run_filter) {
+ /*
+ * bpf_jit_free() can be called from softirq; module_free()
+ * requires process context.
+ */
struct work_struct *work = (struct work_struct *)fp->bpf_func;
INIT_WORK(work, jit_free_defer);
diff -puN arch/s390/net/bpf_jit_comp.c~a arch/s390/net/bpf_jit_comp.c
--- a/arch/s390/net/bpf_jit_comp.c~a
+++ a/arch/s390/net/bpf_jit_comp.c
@@ -818,6 +818,10 @@ void bpf_jit_free(struct sk_filter *fp)
if (fp->bpf_func == sk_run_filter)
return;
+ /*
+ * bpf_jit_free() can be called from softirq; module_free() requires
+ * process context.
+ */
work = (struct work_struct *)fp->bpf_func;
INIT_WORK(work, jit_free_defer);
schedule_work(work);
diff -puN arch/x86/net/bpf_jit_comp.c~a arch/x86/net/bpf_jit_comp.c
--- a/arch/x86/net/bpf_jit_comp.c~a
+++ a/arch/x86/net/bpf_jit_comp.c
@@ -749,6 +749,10 @@ static void jit_free_defer(struct work_s
void bpf_jit_free(struct sk_filter *fp)
{
if (fp->bpf_func != sk_run_filter) {
+ /*
+ * bpf_jit_free() can be called from softirq; module_free()
+ * requires process context.
+ */
struct work_struct *work = (struct work_struct *)fp->bpf_func;
INIT_WORK(work, jit_free_defer);
diff -puN include/linux/filter.h~a include/linux/filter.h
diff -puN net/core/filter.c~a net/core/filter.c
_
next prev parent reply other threads:[~2013-04-24 22:33 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
2013-04-24 15:52 ` Nicolas Schichan
2013-04-24 22:33 ` Andrew Morton [this message]
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=20130424153323.73901c8e026580dc279329b0@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
all inboxes | Powered by JetHome®