Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
From: Hannes Frederic Sowa
Date: Tue, 2 Sep 2014 22:53:44 +0200
> From: Daniel Borkmann
>
> With eBPF getting more extended and exposure to user space is on it's way,
> hardening the memory range the interpreter uses to steer its command flow
> seems appropriate. This patch moves the to be interpreted bytecode to
> read-only pages.
>
> In case we execute a corrupted BPF interpreter image for some reason e.g.
> caused by an attacker which got past a verifier stage, it would not only
> provide arbitrary read/write memory access but arbitrary function calls
> as well. After setting up the BPF interpreter image, its contents do not
> change until destruction time, thus we can setup the image on immutable
> made pages in order to mitigate modifications to that code. The idea
> is derived from commit 314beb9bcabf ("x86: bpf_jit_comp: secure bpf jit
> against spraying attacks").
>
> This is possible because bpf_prog is not part of sk_filter anymore.
> After setup bpf_prog cannot be altered during its life-time. This prevents
> any modifications to the entire bpf_prog structure (incl. function/JIT
> image pointer).
>
> Every eBPF program (including classic BPF that are migrated) have to call
> bpf_prog_select_runtime() to select either interpreter or a JIT image
> as a last setup step, and they all are being freed via bpf_prog_free(),
> including non-JIT. Therefore, we can easily integrate this into the
> eBPF life-time, plus since we directly allocate a bpf_prog, we have no
> performance penalty.
>
> Tested with seccomp and test_bpf testsuite in JIT/non-JIT mode and manual
> inspection of kernel_page_tables. Brad Spengler proposed the same idea
> via Twitter during development of this patch.
>
> Joint work with Hannes Frederic Sowa.
>
> Suggested-by: Brad Spengler
> Signed-off-by: Daniel Borkmann
> Signed-off-by: Hannes Frederic Sowa
Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
From: Hannes Frederic Sowa han...@stressinduktion.org Date: Tue, 2 Sep 2014 22:53:44 +0200 From: Daniel Borkmann dbork...@redhat.com With eBPF getting more extended and exposure to user space is on it's way, hardening the memory range the interpreter uses to steer its command flow seems appropriate. This patch moves the to be interpreted bytecode to read-only pages. In case we execute a corrupted BPF interpreter image for some reason e.g. caused by an attacker which got past a verifier stage, it would not only provide arbitrary read/write memory access but arbitrary function calls as well. After setting up the BPF interpreter image, its contents do not change until destruction time, thus we can setup the image on immutable made pages in order to mitigate modifications to that code. The idea is derived from commit 314beb9bcabf (x86: bpf_jit_comp: secure bpf jit against spraying attacks). This is possible because bpf_prog is not part of sk_filter anymore. After setup bpf_prog cannot be altered during its life-time. This prevents any modifications to the entire bpf_prog structure (incl. function/JIT image pointer). Every eBPF program (including classic BPF that are migrated) have to call bpf_prog_select_runtime() to select either interpreter or a JIT image as a last setup step, and they all are being freed via bpf_prog_free(), including non-JIT. Therefore, we can easily integrate this into the eBPF life-time, plus since we directly allocate a bpf_prog, we have no performance penalty. Tested with seccomp and test_bpf testsuite in JIT/non-JIT mode and manual inspection of kernel_page_tables. Brad Spengler proposed the same idea via Twitter during development of this patch. Joint work with Hannes Frederic Sowa. Suggested-by: Brad Spengler spen...@grsecurity.net Signed-off-by: Daniel Borkmann dbork...@redhat.com Signed-off-by: Hannes Frederic Sowa han...@stressinduktion.org Applied, thanks. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
On Tue, Sep 2, 2014 at 2:43 PM, Hannes Frederic Sowa
wrote:
> On Tue, Sep 2, 2014, at 23:40, Eric Dumazet wrote:
>> On Tue, 2014-09-02 at 14:31 -0700, Alexei Starovoitov wrote:
>>
>> > > +static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
>> > > +{
>> > > + set_memory_rw((unsigned long)fp, fp->pages);
>> >
>> > why rw is needed?
>> > since fp is allocated with vmalloc, vfree doesn't need
>> > to touch the pages to free them, no?
>>
>> That assumes that vmalloc() do not have any debugging features, like
>> poisoning content before freeing, to catch some use after free.
>>
>> Lets be clean and safe, and give back same memory permission we had
>> after vmalloc()
>
> Yes, I agree. I just went down the kmemleak codepaths and we certainly
> don't want to cause issues in there if the implementation changes one
> day.
agree.
I asked, because skipping set_memory_rw() would have
removed the need for 'struct bpf_work_struct' and complexity
around it.
Simple testing looks good, so:
Acked-by: Alexei Starovoitov
will rebase with all of my stuff and do some more tests.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
On 09/02/2014 11:31 PM, Alexei Starovoitov wrote:
...
+#ifdef CONFIG_DEBUG_SET_MODULE_RONX
+static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
+{
+ set_memory_ro((unsigned long)fp, fp->pages);
since ronx are ifdef checked together,
would probably make sense to set nx too?
In case of JITs, for example, we request pages that are
PAGE_KERNEL_EXEC via module_alloc(), but here we only need
PAGE_KERNEL. At least on x86_64, PAGE_NX is then set already.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
On Tue, Sep 2, 2014, at 23:40, Eric Dumazet wrote:
> On Tue, 2014-09-02 at 14:31 -0700, Alexei Starovoitov wrote:
>
> > > +static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
> > > +{
> > > + set_memory_rw((unsigned long)fp, fp->pages);
> >
> > why rw is needed?
> > since fp is allocated with vmalloc, vfree doesn't need
> > to touch the pages to free them, no?
>
> That assumes that vmalloc() do not have any debugging features, like
> poisoning content before freeing, to catch some use after free.
>
> Lets be clean and safe, and give back same memory permission we had
> after vmalloc()
Yes, I agree. I just went down the kmemleak codepaths and we certainly
don't want to cause issues in there if the implementation changes one
day.
Bye,
Hannes
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
On Tue, 2014-09-02 at 14:31 -0700, Alexei Starovoitov wrote:
> > +static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
> > +{
> > + set_memory_rw((unsigned long)fp, fp->pages);
>
> why rw is needed?
> since fp is allocated with vmalloc, vfree doesn't need
> to touch the pages to free them, no?
That assumes that vmalloc() do not have any debugging features, like
poisoning content before freeing, to catch some use after free.
Lets be clean and safe, and give back same memory permission we had
after vmalloc()
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
On Tue, Sep 2, 2014, at 23:31, Alexei Starovoitov wrote:
> On Tue, Sep 2, 2014 at 1:53 PM, Hannes Frederic Sowa
> wrote:
> > From: Daniel Borkmann
> >
> > With eBPF getting more extended and exposure to user space is on it's way,
> > hardening the memory range the interpreter uses to steer its command flow
> > seems appropriate. This patch moves the to be interpreted bytecode to
> > read-only pages.
> ...
> > 11 files changed, 144 insertions(+), 32 deletions(-)
>
> nice. quite short.
>
> > +#ifdef CONFIG_DEBUG_SET_MODULE_RONX
> > +static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
> > +{
> > + set_memory_ro((unsigned long)fp, fp->pages);
>
> since ronx are ifdef checked together,
> would probably make sense to set nx too?
NX bit is already set, because we didn't request page with
PAGE_KERNEL_EXEC.
E.g. in kernel_page_tables:
0xc9a94000-0xc9a96000 8K ro
GLB NX pte
> > +static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
> > +{
> > + set_memory_rw((unsigned long)fp, fp->pages);
>
> why rw is needed?
> since fp is allocated with vmalloc, vfree doesn't need
> to touch the pages to free them, no?
We will check that. It basically was copied from jit hardening code.
Maybe we can omit the call.
Thanks,
Hannes
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
On Tue, Sep 2, 2014 at 1:53 PM, Hannes Frederic Sowa
wrote:
> From: Daniel Borkmann
>
> With eBPF getting more extended and exposure to user space is on it's way,
> hardening the memory range the interpreter uses to steer its command flow
> seems appropriate. This patch moves the to be interpreted bytecode to
> read-only pages.
...
> 11 files changed, 144 insertions(+), 32 deletions(-)
nice. quite short.
> +#ifdef CONFIG_DEBUG_SET_MODULE_RONX
> +static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
> +{
> + set_memory_ro((unsigned long)fp, fp->pages);
since ronx are ifdef checked together,
would probably make sense to set nx too?
> +static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
> +{
> + set_memory_rw((unsigned long)fp, fp->pages);
why rw is needed?
since fp is allocated with vmalloc, vfree doesn't need
to touch the pages to free them, no?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
From: Daniel Borkmann
With eBPF getting more extended and exposure to user space is on it's way,
hardening the memory range the interpreter uses to steer its command flow
seems appropriate. This patch moves the to be interpreted bytecode to
read-only pages.
In case we execute a corrupted BPF interpreter image for some reason e.g.
caused by an attacker which got past a verifier stage, it would not only
provide arbitrary read/write memory access but arbitrary function calls
as well. After setting up the BPF interpreter image, its contents do not
change until destruction time, thus we can setup the image on immutable
made pages in order to mitigate modifications to that code. The idea
is derived from commit 314beb9bcabf ("x86: bpf_jit_comp: secure bpf jit
against spraying attacks").
This is possible because bpf_prog is not part of sk_filter anymore.
After setup bpf_prog cannot be altered during its life-time. This prevents
any modifications to the entire bpf_prog structure (incl. function/JIT
image pointer).
Every eBPF program (including classic BPF that are migrated) have to call
bpf_prog_select_runtime() to select either interpreter or a JIT image
as a last setup step, and they all are being freed via bpf_prog_free(),
including non-JIT. Therefore, we can easily integrate this into the
eBPF life-time, plus since we directly allocate a bpf_prog, we have no
performance penalty.
Tested with seccomp and test_bpf testsuite in JIT/non-JIT mode and manual
inspection of kernel_page_tables. Brad Spengler proposed the same idea
via Twitter during development of this patch.
Joint work with Hannes Frederic Sowa.
Suggested-by: Brad Spengler
Signed-off-by: Daniel Borkmann
Signed-off-by: Hannes Frederic Sowa
Cc: Alexei Starovoitov
Cc: Kees Cook
---
v2) During proofreading I accidentally did not remove the old duplicate
paragraph from the changelog.
arch/arm/net/bpf_jit_32.c | 3 +-
arch/mips/net/bpf_jit.c | 3 +-
arch/powerpc/net/bpf_jit_comp.c | 3 +-
arch/s390/net/bpf_jit_comp.c| 2 +-
arch/sparc/net/bpf_jit_comp.c | 3 +-
arch/x86/net/bpf_jit_comp.c | 18 --
include/linux/filter.h | 49 ++---
kernel/bpf/core.c | 80 +++--
kernel/seccomp.c| 7 ++--
lib/test_bpf.c | 2 +-
net/core/filter.c | 6 ++--
11 files changed, 144 insertions(+), 32 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index a37b989..a76623b 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -930,5 +930,6 @@ void bpf_jit_free(struct bpf_prog *fp)
{
if (fp->jited)
module_free(NULL, fp->bpf_func);
- kfree(fp);
+
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/mips/net/bpf_jit.c b/arch/mips/net/bpf_jit.c
index 05a5661..cfa83cf 100644
--- a/arch/mips/net/bpf_jit.c
+++ b/arch/mips/net/bpf_jit.c
@@ -1427,5 +1427,6 @@ void bpf_jit_free(struct bpf_prog *fp)
{
if (fp->jited)
module_free(NULL, fp->bpf_func);
- kfree(fp);
+
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 3afa6f4..40c53ff 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -697,5 +697,6 @@ void bpf_jit_free(struct bpf_prog *fp)
{
if (fp->jited)
module_free(NULL, fp->bpf_func);
- kfree(fp);
+
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 61e45b7..f2833c5 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -887,5 +887,5 @@ void bpf_jit_free(struct bpf_prog *fp)
module_free(NULL, header);
free_filter:
- kfree(fp);
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/sparc/net/bpf_jit_comp.c b/arch/sparc/net/bpf_jit_comp.c
index 1f76c22..f7a736b 100644
--- a/arch/sparc/net/bpf_jit_comp.c
+++ b/arch/sparc/net/bpf_jit_comp.c
@@ -812,5 +812,6 @@ void bpf_jit_free(struct bpf_prog *fp)
{
if (fp->jited)
module_free(NULL, fp->bpf_func);
- kfree(fp);
+
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index b08a98c..39ccfbb 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -972,23 +972,17 @@ out:
kfree(addrs);
}
-static void bpf_jit_free_deferred(struct work_struct *work)
+void bpf_jit_free(struct bpf_prog *fp)
{
- struct bpf_prog *fp = container_of(work, struct bpf_prog, work);
unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK;
struct bpf_binary_header *header = (void *)addr;
+ if (!fp->jited)
+ goto free_filter;
+
set_memory_rw(addr, header->pages);
module_free(NULL, header);
- kfree(fp);
-}
-void bpf_jit_free(struct bpf_prog *fp)
-{
- if
[PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
From: Daniel Borkmann dbork...@redhat.com
With eBPF getting more extended and exposure to user space is on it's way,
hardening the memory range the interpreter uses to steer its command flow
seems appropriate. This patch moves the to be interpreted bytecode to
read-only pages.
In case we execute a corrupted BPF interpreter image for some reason e.g.
caused by an attacker which got past a verifier stage, it would not only
provide arbitrary read/write memory access but arbitrary function calls
as well. After setting up the BPF interpreter image, its contents do not
change until destruction time, thus we can setup the image on immutable
made pages in order to mitigate modifications to that code. The idea
is derived from commit 314beb9bcabf (x86: bpf_jit_comp: secure bpf jit
against spraying attacks).
This is possible because bpf_prog is not part of sk_filter anymore.
After setup bpf_prog cannot be altered during its life-time. This prevents
any modifications to the entire bpf_prog structure (incl. function/JIT
image pointer).
Every eBPF program (including classic BPF that are migrated) have to call
bpf_prog_select_runtime() to select either interpreter or a JIT image
as a last setup step, and they all are being freed via bpf_prog_free(),
including non-JIT. Therefore, we can easily integrate this into the
eBPF life-time, plus since we directly allocate a bpf_prog, we have no
performance penalty.
Tested with seccomp and test_bpf testsuite in JIT/non-JIT mode and manual
inspection of kernel_page_tables. Brad Spengler proposed the same idea
via Twitter during development of this patch.
Joint work with Hannes Frederic Sowa.
Suggested-by: Brad Spengler spen...@grsecurity.net
Signed-off-by: Daniel Borkmann dbork...@redhat.com
Signed-off-by: Hannes Frederic Sowa han...@stressinduktion.org
Cc: Alexei Starovoitov a...@plumgrid.com
Cc: Kees Cook keesc...@chromium.org
---
v2) During proofreading I accidentally did not remove the old duplicate
paragraph from the changelog.
arch/arm/net/bpf_jit_32.c | 3 +-
arch/mips/net/bpf_jit.c | 3 +-
arch/powerpc/net/bpf_jit_comp.c | 3 +-
arch/s390/net/bpf_jit_comp.c| 2 +-
arch/sparc/net/bpf_jit_comp.c | 3 +-
arch/x86/net/bpf_jit_comp.c | 18 --
include/linux/filter.h | 49 ++---
kernel/bpf/core.c | 80 +++--
kernel/seccomp.c| 7 ++--
lib/test_bpf.c | 2 +-
net/core/filter.c | 6 ++--
11 files changed, 144 insertions(+), 32 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index a37b989..a76623b 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -930,5 +930,6 @@ void bpf_jit_free(struct bpf_prog *fp)
{
if (fp-jited)
module_free(NULL, fp-bpf_func);
- kfree(fp);
+
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/mips/net/bpf_jit.c b/arch/mips/net/bpf_jit.c
index 05a5661..cfa83cf 100644
--- a/arch/mips/net/bpf_jit.c
+++ b/arch/mips/net/bpf_jit.c
@@ -1427,5 +1427,6 @@ void bpf_jit_free(struct bpf_prog *fp)
{
if (fp-jited)
module_free(NULL, fp-bpf_func);
- kfree(fp);
+
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 3afa6f4..40c53ff 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -697,5 +697,6 @@ void bpf_jit_free(struct bpf_prog *fp)
{
if (fp-jited)
module_free(NULL, fp-bpf_func);
- kfree(fp);
+
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 61e45b7..f2833c5 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -887,5 +887,5 @@ void bpf_jit_free(struct bpf_prog *fp)
module_free(NULL, header);
free_filter:
- kfree(fp);
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/sparc/net/bpf_jit_comp.c b/arch/sparc/net/bpf_jit_comp.c
index 1f76c22..f7a736b 100644
--- a/arch/sparc/net/bpf_jit_comp.c
+++ b/arch/sparc/net/bpf_jit_comp.c
@@ -812,5 +812,6 @@ void bpf_jit_free(struct bpf_prog *fp)
{
if (fp-jited)
module_free(NULL, fp-bpf_func);
- kfree(fp);
+
+ bpf_prog_unlock_free(fp);
}
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index b08a98c..39ccfbb 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -972,23 +972,17 @@ out:
kfree(addrs);
}
-static void bpf_jit_free_deferred(struct work_struct *work)
+void bpf_jit_free(struct bpf_prog *fp)
{
- struct bpf_prog *fp = container_of(work, struct bpf_prog, work);
unsigned long addr = (unsigned long)fp-bpf_func PAGE_MASK;
struct bpf_binary_header *header = (void *)addr;
+ if (!fp-jited)
+ goto free_filter;
+
set_memory_rw(addr, header-pages);
Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
On Tue, Sep 2, 2014 at 1:53 PM, Hannes Frederic Sowa
han...@stressinduktion.org wrote:
From: Daniel Borkmann dbork...@redhat.com
With eBPF getting more extended and exposure to user space is on it's way,
hardening the memory range the interpreter uses to steer its command flow
seems appropriate. This patch moves the to be interpreted bytecode to
read-only pages.
...
11 files changed, 144 insertions(+), 32 deletions(-)
nice. quite short.
+#ifdef CONFIG_DEBUG_SET_MODULE_RONX
+static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
+{
+ set_memory_ro((unsigned long)fp, fp-pages);
since ronx are ifdef checked together,
would probably make sense to set nx too?
+static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
+{
+ set_memory_rw((unsigned long)fp, fp-pages);
why rw is needed?
since fp is allocated with vmalloc, vfree doesn't need
to touch the pages to free them, no?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
On Tue, Sep 2, 2014, at 23:31, Alexei Starovoitov wrote:
On Tue, Sep 2, 2014 at 1:53 PM, Hannes Frederic Sowa
han...@stressinduktion.org wrote:
From: Daniel Borkmann dbork...@redhat.com
With eBPF getting more extended and exposure to user space is on it's way,
hardening the memory range the interpreter uses to steer its command flow
seems appropriate. This patch moves the to be interpreted bytecode to
read-only pages.
...
11 files changed, 144 insertions(+), 32 deletions(-)
nice. quite short.
+#ifdef CONFIG_DEBUG_SET_MODULE_RONX
+static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
+{
+ set_memory_ro((unsigned long)fp, fp-pages);
since ronx are ifdef checked together,
would probably make sense to set nx too?
NX bit is already set, because we didn't request page with
PAGE_KERNEL_EXEC.
E.g. in kernel_page_tables:
0xc9a94000-0xc9a96000 8K ro
GLB NX pte
+static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
+{
+ set_memory_rw((unsigned long)fp, fp-pages);
why rw is needed?
since fp is allocated with vmalloc, vfree doesn't need
to touch the pages to free them, no?
We will check that. It basically was copied from jit hardening code.
Maybe we can omit the call.
Thanks,
Hannes
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
On Tue, 2014-09-02 at 14:31 -0700, Alexei Starovoitov wrote:
+static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
+{
+ set_memory_rw((unsigned long)fp, fp-pages);
why rw is needed?
since fp is allocated with vmalloc, vfree doesn't need
to touch the pages to free them, no?
That assumes that vmalloc() do not have any debugging features, like
poisoning content before freeing, to catch some use after free.
Lets be clean and safe, and give back same memory permission we had
after vmalloc()
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
On Tue, Sep 2, 2014, at 23:40, Eric Dumazet wrote:
On Tue, 2014-09-02 at 14:31 -0700, Alexei Starovoitov wrote:
+static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
+{
+ set_memory_rw((unsigned long)fp, fp-pages);
why rw is needed?
since fp is allocated with vmalloc, vfree doesn't need
to touch the pages to free them, no?
That assumes that vmalloc() do not have any debugging features, like
poisoning content before freeing, to catch some use after free.
Lets be clean and safe, and give back same memory permission we had
after vmalloc()
Yes, I agree. I just went down the kmemleak codepaths and we certainly
don't want to cause issues in there if the implementation changes one
day.
Bye,
Hannes
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
On 09/02/2014 11:31 PM, Alexei Starovoitov wrote:
...
+#ifdef CONFIG_DEBUG_SET_MODULE_RONX
+static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
+{
+ set_memory_ro((unsigned long)fp, fp-pages);
since ronx are ifdef checked together,
would probably make sense to set nx too?
In case of JITs, for example, we request pages that are
PAGE_KERNEL_EXEC via module_alloc(), but here we only need
PAGE_KERNEL. At least on x86_64, PAGE_NX is then set already.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH net-next v2] net: bpf: make eBPF interpreter images read-only
On Tue, Sep 2, 2014 at 2:43 PM, Hannes Frederic Sowa
han...@stressinduktion.org wrote:
On Tue, Sep 2, 2014, at 23:40, Eric Dumazet wrote:
On Tue, 2014-09-02 at 14:31 -0700, Alexei Starovoitov wrote:
+static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
+{
+ set_memory_rw((unsigned long)fp, fp-pages);
why rw is needed?
since fp is allocated with vmalloc, vfree doesn't need
to touch the pages to free them, no?
That assumes that vmalloc() do not have any debugging features, like
poisoning content before freeing, to catch some use after free.
Lets be clean and safe, and give back same memory permission we had
after vmalloc()
Yes, I agree. I just went down the kmemleak codepaths and we certainly
don't want to cause issues in there if the implementation changes one
day.
agree.
I asked, because skipping set_memory_rw() would have
removed the need for 'struct bpf_work_struct' and complexity
around it.
Simple testing looks good, so:
Acked-by: Alexei Starovoitov a...@plumgrid.com
will rebase with all of my stuff and do some more tests.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
