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: 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
_


  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®