Discussion:
[PATCH 0/4] Seccomp filter JIT support on ARM.
(too old to reply)
Nicolas Schichan
2015-04-29 13:40:02 UTC
Permalink
Greetings,

The following patches allow the use of the existing JIT code under
arch/arm for seccomp filters.

The first patch makes bpf_migrate_filter() available so that seccomp
code can use it.

The second patch invokes the classic BPF JIT code and if this fails
reverts to the internal BPF. The open coded double call to
bpf_convert_filter() and bpf_prog_select_runtime() is replaced by
bpf_migrate_filter()

The third patch adds support for loads from the seccomp_data structure in
the ARM BPF JIT code.

The fourth and final patch fixes a bug in the emit_udiv() function
when used to convert a BPF_ALU | BPF_DIV | BPF_K instruction in the
ARM BPF JIT code.

Regards,

Nicolas Schichan (4):
net: filter: make bpf_migrate_filter available outside of
net/core/filter.c
seccomp: rework seccomp_prepare_filter().
ARM: net: add JIT support for loads from struct seccomp_data.
ARM: net fix emit_udiv() for BPF_ALU | BPF_DIV | BPF_K intruction.

arch/arm/net/bpf_jit_32.c | 14 ++++++++++++--
include/linux/filter.h | 1 +
kernel/seccomp.c | 47 ++++++++++++++++++++---------------------------
net/core/filter.c | 2 +-
4 files changed, 34 insertions(+), 30 deletions(-)
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Nicolas Schichan
2015-04-29 13:40:02 UTC
Permalink
In that case, emit_udiv() will be called with rn == ARM_R0 (r_scratch)
and loading rm first into ARM_R0 will result in jit_udiv() function
being called the same dividend and divisor. Fix that by loading rn
first into ARM_R1 and then rm into ARM_R0.

Signed-off-by: Nicolas Schichan <***@freebox.fr>
---
arch/arm/net/bpf_jit_32.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index b5f470d..ffaf311 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -449,10 +449,10 @@ static inline void emit_udiv(u8 rd, u8 rm, u8 rn, struct jit_ctx *ctx)
return;
}
#endif
- if (rm != ARM_R0)
- emit(ARM_MOV_R(ARM_R0, rm), ctx);
if (rn != ARM_R1)
emit(ARM_MOV_R(ARM_R1, rn), ctx);
+ if (rm != ARM_R0)
+ emit(ARM_MOV_R(ARM_R0, rm), ctx);

ctx->seen |= SEEN_CALL;
emit_mov_i(ARM_R3, (u32)jit_udiv, ctx);
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Russell King - ARM Linux
2015-05-01 17:40:02 UTC
Permalink
Post by Nicolas Schichan
In that case, emit_udiv() will be called with rn == ARM_R0 (r_scratch)
and loading rm first into ARM_R0 will result in jit_udiv() function
being called the same dividend and divisor. Fix that by loading rn
first into ARM_R1 and then rm into ARM_R0.
---
arch/arm/net/bpf_jit_32.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index b5f470d..ffaf311 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -449,10 +449,10 @@ static inline void emit_udiv(u8 rd, u8 rm, u8 rn, struct jit_ctx *ctx)
return;
}
#endif
- if (rm != ARM_R0)
- emit(ARM_MOV_R(ARM_R0, rm), ctx);
if (rn != ARM_R1)
emit(ARM_MOV_R(ARM_R1, rn), ctx);
+ if (rm != ARM_R0)
+ emit(ARM_MOV_R(ARM_R0, rm), ctx);
I don't think you've thought enough about this. What if rm is ARM_R1?
What if rn = ARM_R0 and rm = ARM_R1?

How about:

if (rn == ARM_R0 && rm == ARM_R1) {
emit(ARM_MOV_R(ARM_R3, rn), ctx); // r3 <- r0(rn)
emit(ARM_MOV_R(ARM_R0, rm), ctx); // r0 <- r1(rm)
emit(ARM_MOV_R(ARM_R1, ARM_R3), ctx); // r1 <- r3
} else if (rn == ARM_R0) {
emit(ARM_MOV_R(ARM_R1, rn), ctx); // r1 <- rn
if (rm != ARM_R0)
emit(ARM_MOV_R(ARM_R0, rm), ctx); // r0 <- rm
} else {
if (rm != ARM_R0)
emit(ARM_MOV_R(ARM_R0, rm), ctx); // r0 <- rm
if (rn != ARM_R1)
emit(ARM_MOV_R(ARM_R1, rn), ctx); // r1 <- rn
}
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Nicolas Schichan
2015-05-04 16:20:02 UTC
Permalink
[...]
Post by Russell King - ARM Linux
Post by Nicolas Schichan
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index b5f470d..ffaf311 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -449,10 +449,10 @@ static inline void emit_udiv(u8 rd, u8 rm, u8 rn, struct jit_ctx *ctx)
return;
}
#endif
- if (rm != ARM_R0)
- emit(ARM_MOV_R(ARM_R0, rm), ctx);
if (rn != ARM_R1)
emit(ARM_MOV_R(ARM_R1, rn), ctx);
+ if (rm != ARM_R0)
+ emit(ARM_MOV_R(ARM_R0, rm), ctx);
I don't think you've thought enough about this. What if rm is ARM_R1?
What if rn = ARM_R0 and rm = ARM_R1?
if (rn == ARM_R0 && rm == ARM_R1) {
emit(ARM_MOV_R(ARM_R3, rn), ctx); // r3 <- r0(rn)
emit(ARM_MOV_R(ARM_R0, rm), ctx); // r0 <- r1(rm)
emit(ARM_MOV_R(ARM_R1, ARM_R3), ctx); // r1 <- r3
} else if (rn == ARM_R0) {
emit(ARM_MOV_R(ARM_R1, rn), ctx); // r1 <- rn
if (rm != ARM_R0)
emit(ARM_MOV_R(ARM_R0, rm), ctx); // r0 <- rm
} else {
if (rm != ARM_R0)
emit(ARM_MOV_R(ARM_R0, rm), ctx); // r0 <- rm
if (rn != ARM_R1)
emit(ARM_MOV_R(ARM_R1, rn), ctx); // r1 <- rn
}
Hello Russell,

In the current JIT, emit_udiv() is only being called with:

- rm = ARM_R4 (r_A) and rn = ARM_R0 (r_scrach) for BPF_ALU | BPF_DIV | BPF_K

- rm = ARM_R4 (r_A) and rn = ARM_R5 (r_X) for BPF_ALU | BPF_DIV | BPF_X

so it should not cause any issue in the current code state.

But yes, I'll rework the patch to avoid any other nasty surprises should the
code change.

Thanks,
--
Nicolas Schichan
Freebox SAS
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Russell King - ARM Linux
2015-05-04 18:00:02 UTC
Permalink
Post by Nicolas Schichan
[...]
Post by Russell King - ARM Linux
Post by Nicolas Schichan
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index b5f470d..ffaf311 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -449,10 +449,10 @@ static inline void emit_udiv(u8 rd, u8 rm, u8 rn, struct jit_ctx *ctx)
return;
}
#endif
- if (rm != ARM_R0)
- emit(ARM_MOV_R(ARM_R0, rm), ctx);
if (rn != ARM_R1)
emit(ARM_MOV_R(ARM_R1, rn), ctx);
+ if (rm != ARM_R0)
+ emit(ARM_MOV_R(ARM_R0, rm), ctx);
I don't think you've thought enough about this. What if rm is ARM_R1?
What if rn = ARM_R0 and rm = ARM_R1?
if (rn == ARM_R0 && rm == ARM_R1) {
emit(ARM_MOV_R(ARM_R3, rn), ctx); // r3 <- r0(rn)
emit(ARM_MOV_R(ARM_R0, rm), ctx); // r0 <- r1(rm)
emit(ARM_MOV_R(ARM_R1, ARM_R3), ctx); // r1 <- r3
} else if (rn == ARM_R0) {
emit(ARM_MOV_R(ARM_R1, rn), ctx); // r1 <- rn
if (rm != ARM_R0)
emit(ARM_MOV_R(ARM_R0, rm), ctx); // r0 <- rm
} else {
if (rm != ARM_R0)
emit(ARM_MOV_R(ARM_R0, rm), ctx); // r0 <- rm
if (rn != ARM_R1)
emit(ARM_MOV_R(ARM_R1, rn), ctx); // r1 <- rn
}
Hello Russell,
- rm = ARM_R4 (r_A) and rn = ARM_R0 (r_scrach) for BPF_ALU | BPF_DIV | BPF_K
- rm = ARM_R4 (r_A) and rn = ARM_R5 (r_X) for BPF_ALU | BPF_DIV | BPF_X
so it should not cause any issue in the current code state.
But yes, I'll rework the patch to avoid any other nasty surprises should the
code change.
Maybe then add a comment detailing the current conditions that this is
coded for so that if you're not around when the code changes, others are
aware of the issue.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Nicolas Schichan
2015-04-29 13:40:01 UTC
Permalink
Signed-off-by: Nicolas Schichan <***@freebox.fr>
---
arch/arm/net/bpf_jit_32.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index e1268f9..b5f470d 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -852,6 +852,16 @@ b_epilogue:
off = offsetof(struct sk_buff, queue_mapping);
emit(ARM_LDRH_I(r_A, r_skb, off), ctx);
break;
+ case BPF_LDX | BPF_W | BPF_ABS:
+ /*
+ * load a 32bit word from struct seccomp_data.
+ * seccomp_check_filter() will already have checked
+ * that k is 32bit aligned and lies within the
+ * struct seccomp_data.
+ */
+ ctx->seen |= SEEN_SKB;
+ emit(ARM_LDR_I(r_A, r_skb, k), ctx);
+ break;
default:
return -1;
}
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Nicolas Schichan
2015-04-29 13:40:01 UTC
Permalink
This is in preparation of its use in the seccomp code.

Signed-off-by: Nicolas Schichan <***@freebox.fr>
---
include/linux/filter.h | 1 +
net/core/filter.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index caac208..7e0811b 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -379,6 +379,7 @@ static inline void bpf_prog_unlock_free(struct bpf_prog *fp)

int bpf_prog_create(struct bpf_prog **pfp, struct sock_fprog_kern *fprog);
void bpf_prog_destroy(struct bpf_prog *fp);
+struct bpf_prog *bpf_migrate_filter(struct bpf_prog *fp);

int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk);
int sk_attach_bpf(u32 ufd, struct sock *sk);
diff --git a/net/core/filter.c b/net/core/filter.c
index f6bdc2b..b8ce66e 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -876,7 +876,7 @@ bool sk_filter_charge(struct sock *sk, struct sk_filter *fp)
return false;
}

-static struct bpf_prog *bpf_migrate_filter(struct bpf_prog *fp)
+struct bpf_prog *bpf_migrate_filter(struct bpf_prog *fp)
{
struct sock_filter *old_prog;
struct bpf_prog *old_fp;
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Nicolas Schichan
2015-04-29 13:40:03 UTC
Permalink
- Try to use the classic BPF JIT via bpf_jit_compile().

- Use bpf_migrate_filter() from NET filter code instead of the double
bpf_convert_filter() followed by bpf_prog_select_runtime() if
classic bpf_jit_compile() did not succeed in producing native code.

Signed-off-by: Nicolas Schichan <***@freebox.fr>
---
kernel/seccomp.c | 47 ++++++++++++++++++++---------------------------
1 file changed, 20 insertions(+), 27 deletions(-)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 4f44028..4f9fea7 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -348,8 +348,7 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
{
struct seccomp_filter *filter;
unsigned long fp_size;
- struct sock_filter *fp;
- int new_len;
+ struct bpf_prog *fp;
long ret;

if (fprog->len == 0 || fprog->len > BPF_MAXINSNS)
@@ -368,29 +367,38 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
CAP_SYS_ADMIN) != 0)
return ERR_PTR(-EACCES);

- fp = kzalloc(fp_size, GFP_KERNEL|__GFP_NOWARN);
+ fp = bpf_prog_alloc(bpf_prog_size(fprog->len), __GFP_NOWARN);
if (!fp)
return ERR_PTR(-ENOMEM);

/* Copy the instructions from fprog. */
ret = -EFAULT;
- if (copy_from_user(fp, fprog->filter, fp_size))
+ if (copy_from_user(fp->insns, fprog->filter, fp_size))
goto free_prog;
+ fp->len = fprog->len;

/* Check and rewrite the fprog via the skb checker */
- ret = bpf_check_classic(fp, fprog->len);
+ ret = bpf_check_classic(fp->insns, fp->len);
if (ret)
goto free_prog;

/* Check and rewrite the fprog for seccomp use */
- ret = seccomp_check_filter(fp, fprog->len);
+ ret = seccomp_check_filter(fp->insns, fp->len);
if (ret)
goto free_prog;

- /* Convert 'sock_filter' insns to 'bpf_insn' insns */
- ret = bpf_convert_filter(fp, fprog->len, NULL, &new_len);
- if (ret)
- goto free_prog;
+ /* try to use the classic JIT */
+ bpf_jit_compile(fp);
+
+ if (!fp->jited) {
+ /*
+ * if classic JIT has failed, try to convert it to an
+ * internal filter
+ */
+ fp = bpf_migrate_filter(fp);
+ if (IS_ERR(fp))
+ return fp;
+ }

/* Allocate a new seccomp_filter */
ret = -ENOMEM;
@@ -399,28 +407,13 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
if (!filter)
goto free_prog;

- filter->prog = bpf_prog_alloc(bpf_prog_size(new_len), __GFP_NOWARN);
- if (!filter->prog)
- goto free_filter;
-
- ret = bpf_convert_filter(fp, fprog->len, filter->prog->insnsi, &new_len);
- if (ret)
- goto free_filter_prog;
-
- kfree(fp);
+ filter->prog = fp;
atomic_set(&filter->usage, 1);
- filter->prog->len = new_len;
-
- bpf_prog_select_runtime(filter->prog);

return filter;

-free_filter_prog:
- __bpf_prog_free(filter->prog);
-free_filter:
- kfree(filter);
free_prog:
- kfree(fp);
+ bpf_prog_destroy(fp);
return ERR_PTR(ret);
}
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Daniel Borkmann
2015-04-29 17:20:03 UTC
Permalink
Post by Nicolas Schichan
- Try to use the classic BPF JIT via bpf_jit_compile().
- Use bpf_migrate_filter() from NET filter code instead of the double
bpf_convert_filter() followed by bpf_prog_select_runtime() if
classic bpf_jit_compile() did not succeed in producing native code.
[ I had to look that one up manually, would be good if you keep
people in Cc, also netdev for BPF in general. ]

I see, you need that to make it available to the old bpf_jit_compile()
for probing on classic JITs. Actually, I really would prefer, if instead
of duplicating that code, you could export bpf_prepare_filter() and
pass seccomp_check_filter() as an argument to bpf_prepare_filter().

Otherwise, in case bpf_prepare_filter() changes, people will easily
forget to update seccomp related code, really.

Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Nicolas Schichan
2015-04-30 12:40:03 UTC
Permalink
Post by Daniel Borkmann
Post by Nicolas Schichan
- Try to use the classic BPF JIT via bpf_jit_compile().
- Use bpf_migrate_filter() from NET filter code instead of the double
bpf_convert_filter() followed by bpf_prog_select_runtime() if
classic bpf_jit_compile() did not succeed in producing native code.
[ I had to look that one up manually, would be good if you keep
people in Cc, also netdev for BPF in general. ]
Hello Daniel,

Sorry about that, I used git-send-email with a --to-cmd set to:

"./scripts/get_maintainer.pl -m --norolestats --git-max-maintainer 2"

and your email didn't show up for this particular patch. Additionally the
other emails in the serie that were addressed to you were addressed to an old
disabled email address of yours.

It didn't show up either without "--git-max-maintainer 2".

I'll take more care about the receiver list for the v2 of this serie.
Post by Daniel Borkmann
I see, you need that to make it available to the old bpf_jit_compile()
for probing on classic JITs. Actually, I really would prefer, if instead
of duplicating that code, you could export bpf_prepare_filter() and
pass seccomp_check_filter() as an argument to bpf_prepare_filter().
Just to be sure you want me to pass a pointer to seccomp_check_filter to
bpf_prepare_filter so that it can run it between bpf_check_classic() and
bpf_jit_compile ?
Post by Daniel Borkmann
Otherwise, in case bpf_prepare_filter() changes, people will easily
forget to update seccomp related code, really.
Fair point.

Thanks,
--
Nicolas Schichan
Freebox SAS
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Daniel Borkmann
2015-04-30 12:50:01 UTC
Permalink
On 04/30/2015 02:27 PM, Nicolas Schichan wrote:
...
Post by Nicolas Schichan
I'll take more care about the receiver list for the v2 of this serie.
Ok, cool.
Post by Nicolas Schichan
Post by Daniel Borkmann
I see, you need that to make it available to the old bpf_jit_compile()
for probing on classic JITs. Actually, I really would prefer, if instead
of duplicating that code, you could export bpf_prepare_filter() and
pass seccomp_check_filter() as an argument to bpf_prepare_filter().
Just to be sure you want me to pass a pointer to seccomp_check_filter to
bpf_prepare_filter so that it can run it between bpf_check_classic() and
bpf_jit_compile ?
For example, what comes to mind is something along these lines:

struct bpf_prog *
bpf_prepare_filter(struct bpf_prog *fp,
int (*aux_trans_classic)(struct sock_filter *filter,
unsigned int flen))
{
int err;

fp->bpf_func = NULL;
fp->jited = false;

err = bpf_check_classic(fp->insns, fp->len);
if (err) {
__bpf_prog_release(fp);
return ERR_PTR(err);
}

/* There might be additional checks and transformations
* needed on classic filters, f.e. in case of seccomp.
*/
if (aux_trans_classic) {
err = aux_trans_classic(fp->insns,
fp->len);
if (err) {
__bpf_prog_release(fp);
return ERR_PTR(err);
}
}

/* Probe if we can JIT compile the filter and if so, do
* the compilation of the filter.
*/
bpf_jit_compile(fp);

/* JIT compiler couldn't process this filter, so do the
* internal BPF translation for the optimized interpreter.
*/
if (!fp->jited)
fp = bpf_migrate_filter(fp);

return fp;
}

From seccomp side, you invoke:

... bpf_prepare_filter(fp, seccomp_check_filter);

I would actually like to see that customized code in seccomp_prepare_filter()
further reduced instead of increased, would certainly make it easier to
maintain and understand.

Do you think something like the above is possible?
Post by Nicolas Schichan
Post by Daniel Borkmann
Otherwise, in case bpf_prepare_filter() changes, people will easily
forget to update seccomp related code, really.
Fair point.
Thanks,
Thanks a lot,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Nicolas Schichan
2015-04-30 14:20:02 UTC
Permalink
Post by Nicolas Schichan
Post by Nicolas Schichan
Just to be sure you want me to pass a pointer to seccomp_check_filter to
bpf_prepare_filter so that it can run it between bpf_check_classic() and
bpf_jit_compile ?
struct bpf_prog *
bpf_prepare_filter(struct bpf_prog *fp,
int (*aux_trans_classic)(struct sock_filter *filter,
unsigned int flen))
{
int err;
fp->bpf_func = NULL;
fp->jited = false;
err = bpf_check_classic(fp->insns, fp->len);
if (err) {
__bpf_prog_release(fp);
return ERR_PTR(err);
}
/* There might be additional checks and transformations
* needed on classic filters, f.e. in case of seccomp.
*/
if (aux_trans_classic) {
err = aux_trans_classic(fp->insns,
fp->len);
if (err) {
__bpf_prog_release(fp);
return ERR_PTR(err);
}
}
/* Probe if we can JIT compile the filter and if so, do
* the compilation of the filter.
*/
bpf_jit_compile(fp);
/* JIT compiler couldn't process this filter, so do the
* internal BPF translation for the optimized interpreter.
*/
if (!fp->jited)
fp = bpf_migrate_filter(fp);
return fp;
}
... bpf_prepare_filter(fp, seccomp_check_filter);
Thanks for the precisions, I'll look into that.
--
Nicolas Schichan
Freebox SAS
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Daniel Borkmann
2015-04-29 16:40:02 UTC
Permalink
On 04/29/2015 03:37 PM, Nicolas Schichan wrote:
...
Post by Nicolas Schichan
The fourth and final patch fixes a bug in the emit_udiv() function
when used to convert a BPF_ALU | BPF_DIV | BPF_K instruction in the
ARM BPF JIT code.
Shouldn't that fix go separately, so it can be included into 4.1
resp. -stable?

Would be good if you also Cc Mircea, who wrote the code. Was that
caught by lib/test_bpf.c suite (if not, would be good to add a test
case for it) ?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Nicolas Schichan
2015-04-30 12:40:01 UTC
Permalink
Post by Daniel Borkmann
...
Post by Nicolas Schichan
The fourth and final patch fixes a bug in the emit_udiv() function
when used to convert a BPF_ALU | BPF_DIV | BPF_K instruction in the
ARM BPF JIT code.
Shouldn't that fix go separately, so it can be included into 4.1
resp. -stable?
Sure, shall I resend that separately from the v2 of the serie or is it fine in
its current form ?
Post by Daniel Borkmann
Would be good if you also Cc Mircea, who wrote the code. Was that
caught by lib/test_bpf.c suite (if not, would be good to add a test
case for it) ?
It was detected by an internal test suite we have here. I see that there are
some BPF_ALU | BPF_DIV | BPF_K instructions so it might also be caught by
lib/test_bpf.c as well.

Regards,
--
Nicolas Schichan
Freebox SAS
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Daniel Borkmann
2015-04-30 13:00:03 UTC
Permalink
Post by Nicolas Schichan
Post by Daniel Borkmann
...
Post by Nicolas Schichan
The fourth and final patch fixes a bug in the emit_udiv() function
when used to convert a BPF_ALU | BPF_DIV | BPF_K instruction in the
ARM BPF JIT code.
Shouldn't that fix go separately, so it can be included into 4.1
resp. -stable?
Sure, shall I resend that separately from the v2 of the serie or is it fine in
its current form ?
Would be great if you could send that as an individual patch, since
it's a stand-alone fix and independent from the (feature) patch set.
Post by Nicolas Schichan
Post by Daniel Borkmann
Would be good if you also Cc Mircea, who wrote the code. Was that
caught by lib/test_bpf.c suite (if not, would be good to add a test
case for it) ?
It was detected by an internal test suite we have here. I see that there are
some BPF_ALU | BPF_DIV | BPF_K instructions so it might also be caught by
lib/test_bpf.c as well.
If there are some additional tests that are not yet covered by lib/test_bpf.c,
I'd be happy if you could add them there. This can also be as a follow-up,
but if we can increase coverage for others as well, the better.

Thanks again,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Alexei Starovoitov
2015-04-30 17:20:03 UTC
Permalink
Post by Daniel Borkmann
If there are some additional tests that are not yet covered by
lib/test_bpf.c,
I'd be happy if you could add them there. This can also be as a follow-up,
but if we can increase coverage for others as well, the better.
btw, Michael have been working on adding 173 new tests to test_bpf.
Michael, could you submit them to net-next ?


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Alexei Starovoitov
2015-04-29 17:00:03 UTC
Permalink
Post by Nicolas Schichan
Greetings,
The following patches allow the use of the existing JIT code under
arch/arm for seccomp filters.
The first patch makes bpf_migrate_filter() available so that seccomp
code can use it.
The second patch invokes the classic BPF JIT code and if this fails
reverts to the internal BPF. The open coded double call to
bpf_convert_filter() and bpf_prog_select_runtime() is replaced by
bpf_migrate_filter()
The third patch adds support for loads from the seccomp_data structure in
the ARM BPF JIT code.
the patches 1,2,3 look fine.
Post by Nicolas Schichan
The fourth and final patch fixes a bug in the emit_udiv() function
when used to convert a BPF_ALU | BPF_DIV | BPF_K instruction in the
ARM BPF JIT code.
this one looks independent from first three and probably should
be sent to stable?

Your cc list is different for every patch, why? What tree are you
thinking to go through? 1st patch is trivial, but 2nd belongs in
seccomp, 3rd and 4th are in arm32... I'd would say arm tree if Kees
is ok with it.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Continue reading on narkive:
Loading...