| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20181219182930.8825-15-jakub.kicinski@netronome.com>
Date: Wed, 19 Dec 2018 10:29:25 -0800
From: Jakub Kicinski <jakub.kicinski@...ronome.com>
To: alexei.starovoitov@...il.com, daniel@...earbox.net
Cc: oss-drivers@...ronome.com, netdev@...r.kernel.org,
Jakub Kicinski <jakub.kicinski@...ronome.com>
Subject: [PATCH bpf-next 14/19] nfp: bpf: split up the skip flag
We fail program loading if jump lands on a skipped instruction.
This is for historical reasons, it used to be that we only skipped
instructions optimized out based on prior context, and therefore
the optimization would be buggy if we jumped directly to such
instruction (because the context would be skipped by the jump).
There are cases where instructions can be skipped without any
context, for example there is no point in generating code for:
r0 |= 0
We will also soon support dropping dead code, so make the skip
logic differentiate between "optimized with preceding context"
vs other skip types.
Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@...ronome.com>
---
drivers/net/ethernet/netronome/nfp/bpf/jit.c | 38 ++++++++++---------
drivers/net/ethernet/netronome/nfp/bpf/main.h | 9 ++++-
2 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
index 284a6a98a4fc..188270c681fc 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
+++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
@@ -1266,7 +1266,7 @@ wrp_alu64_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
u64 imm = insn->imm; /* sign extend */
if (skip) {
- meta->skip = true;
+ meta->flags |= FLAG_INSN_SKIP_NOOP;
return 0;
}
@@ -1296,7 +1296,7 @@ wrp_alu32_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta,
const struct bpf_insn *insn = &meta->insn;
if (skip) {
- meta->skip = true;
+ meta->flags |= FLAG_INSN_SKIP_NOOP;
return 0;
}
@@ -3055,7 +3055,7 @@ static int jset_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
swreg tmp_reg;
if (!imm) {
- meta->skip = true;
+ meta->flags |= FLAG_INSN_SKIP_NOOP;
return 0;
}
@@ -3402,7 +3402,7 @@ static int nfp_fixup_branches(struct nfp_prog *nfp_prog)
int err;
list_for_each_entry(meta, &nfp_prog->insns, l) {
- if (meta->skip)
+ if (meta->flags & FLAG_INSN_SKIP_MASK)
continue;
if (BPF_CLASS(meta->insn.code) != BPF_JMP)
continue;
@@ -3446,7 +3446,7 @@ static int nfp_fixup_branches(struct nfp_prog *nfp_prog)
jmp_dst = meta->jmp_dst;
- if (jmp_dst->skip) {
+ if (jmp_dst->flags & FLAG_INSN_SKIP_PREC_DEPENDENT) {
pr_err("Branch landing on removed instruction!!\n");
return -ELOOP;
}
@@ -3696,7 +3696,7 @@ static int nfp_translate(struct nfp_prog *nfp_prog)
return nfp_prog->error;
}
- if (meta->skip) {
+ if (meta->flags & FLAG_INSN_SKIP_MASK) {
nfp_prog->n_translated++;
continue;
}
@@ -3744,10 +3744,10 @@ static void nfp_bpf_opt_reg_init(struct nfp_prog *nfp_prog)
/* Programs start with R6 = R1 but we ignore the skb pointer */
if (insn.code == (BPF_ALU64 | BPF_MOV | BPF_X) &&
insn.src_reg == 1 && insn.dst_reg == 6)
- meta->skip = true;
+ meta->flags |= FLAG_INSN_SKIP_PREC_DEPENDENT;
/* Return as soon as something doesn't match */
- if (!meta->skip)
+ if (!(meta->flags & FLAG_INSN_SKIP_MASK))
return;
}
}
@@ -3762,7 +3762,7 @@ static void nfp_bpf_opt_neg_add_sub(struct nfp_prog *nfp_prog)
list_for_each_entry(meta, &nfp_prog->insns, l) {
struct bpf_insn insn = meta->insn;
- if (meta->skip)
+ if (meta->flags & FLAG_INSN_SKIP_MASK)
continue;
if (BPF_CLASS(insn.code) != BPF_ALU &&
@@ -3836,7 +3836,7 @@ static void nfp_bpf_opt_ld_mask(struct nfp_prog *nfp_prog)
if (meta2->flags & FLAG_INSN_IS_JUMP_DST)
continue;
- meta2->skip = true;
+ meta2->flags |= FLAG_INSN_SKIP_PREC_DEPENDENT;
}
}
@@ -3876,8 +3876,8 @@ static void nfp_bpf_opt_ld_shift(struct nfp_prog *nfp_prog)
meta3->flags & FLAG_INSN_IS_JUMP_DST)
continue;
- meta2->skip = true;
- meta3->skip = true;
+ meta2->flags |= FLAG_INSN_SKIP_PREC_DEPENDENT;
+ meta3->flags |= FLAG_INSN_SKIP_PREC_DEPENDENT;
}
}
@@ -4072,7 +4072,8 @@ static void nfp_bpf_opt_ldst_gather(struct nfp_prog *nfp_prog)
}
head_ld_meta->paired_st = &head_st_meta->insn;
- head_st_meta->skip = true;
+ head_st_meta->flags |=
+ FLAG_INSN_SKIP_PREC_DEPENDENT;
} else {
head_ld_meta->ldst_gather_len = 0;
}
@@ -4105,8 +4106,8 @@ static void nfp_bpf_opt_ldst_gather(struct nfp_prog *nfp_prog)
head_ld_meta = meta1;
head_st_meta = meta2;
} else {
- meta1->skip = true;
- meta2->skip = true;
+ meta1->flags |= FLAG_INSN_SKIP_PREC_DEPENDENT;
+ meta2->flags |= FLAG_INSN_SKIP_PREC_DEPENDENT;
}
head_ld_meta->ldst_gather_len += BPF_LDST_BYTES(ld);
@@ -4131,7 +4132,7 @@ static void nfp_bpf_opt_pkt_cache(struct nfp_prog *nfp_prog)
if (meta->flags & FLAG_INSN_IS_JUMP_DST)
cache_avail = false;
- if (meta->skip)
+ if (meta->flags & FLAG_INSN_SKIP_MASK)
continue;
insn = &meta->insn;
@@ -4217,7 +4218,7 @@ static void nfp_bpf_opt_pkt_cache(struct nfp_prog *nfp_prog)
}
list_for_each_entry(meta, &nfp_prog->insns, l) {
- if (meta->skip)
+ if (meta->flags & FLAG_INSN_SKIP_MASK)
continue;
if (is_mbpf_load_pkt(meta) && !meta->ldst_gather_len) {
@@ -4253,7 +4254,8 @@ static int nfp_bpf_replace_map_ptrs(struct nfp_prog *nfp_prog)
u32 id;
nfp_for_each_insn_walk2(nfp_prog, meta1, meta2) {
- if (meta1->skip || meta2->skip)
+ if (meta1->flags & FLAG_INSN_SKIP_MASK ||
+ meta2->flags & FLAG_INSN_SKIP_MASK)
continue;
if (meta1->insn.code != (BPF_LD | BPF_IMM | BPF_DW) ||
diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
index 941277936475..40291aedd895 100644
--- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
+++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
@@ -243,6 +243,13 @@ struct nfp_bpf_reg_state {
#define FLAG_INSN_IS_JUMP_DST BIT(0)
#define FLAG_INSN_IS_SUBPROG_START BIT(1)
#define FLAG_INSN_PTR_CALLER_STACK_FRAME BIT(2)
+/* Instruction is pointless, noop even on its own */
+#define FLAG_INSN_SKIP_NOOP BIT(3)
+/* Instruction is optimized out based on preceding instructions */
+#define FLAG_INSN_SKIP_PREC_DEPENDENT BIT(4)
+
+#define FLAG_INSN_SKIP_MASK (FLAG_INSN_SKIP_NOOP | \
+ FLAG_INSN_SKIP_PREC_DEPENDENT)
/**
* struct nfp_insn_meta - BPF instruction wrapper
@@ -271,7 +278,6 @@ struct nfp_bpf_reg_state {
* @n: eBPF instruction number
* @flags: eBPF instruction extra optimization flags
* @subprog_idx: index of subprogram to which the instruction belongs
- * @skip: skip this instruction (optimized out)
* @double_cb: callback for second part of the instruction
* @l: link on nfp_prog->insns list
*/
@@ -319,7 +325,6 @@ struct nfp_insn_meta {
unsigned short n;
unsigned short flags;
unsigned short subprog_idx;
- bool skip;
instr_cb_t double_cb;
struct list_head l;
--
2.19.2
Powered by blists - more mailing lists