Commit fad73a1a authored by Martin KaFai Lau's avatar Martin KaFai Lau Committed by David S. Miller

bpf: Fix and simplifications on inline map lookup

Fix in verifier:
For the same bpf_map_lookup_elem() instruction (i.e. "call 1"),
a broken case is "a different type of map could be used for the
same lookup instruction". For example, an array in one case and a
hashmap in another.  We have to resort to the old dynamic call behavior
in this case.  The fix is to check for collision on insn_aux->map_ptr.
If there is collision, don't inline the map lookup.

Please see the "do_reg_lookup()" in test_map_in_map_kern.c in the later
patch for how-to trigger the above case.

Simplifications on array_map_gen_lookup():
1. Calculate elem_size from map->value_size.  It removes the
   need for 'struct bpf_array' which makes the later map-in-map
   implementation easier.
2. Remove the 'elem_size == 1' test

Fixes: 81ed18ab ("bpf: add helper inlining infra and optimize map_array lookup")
Signed-off-by: default avatarMartin KaFai Lau <kafai@fb.com>
Acked-by: default avatarAlexei Starovoitov <ast@kernel.org>
Acked-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent b4f0a661
...@@ -117,20 +117,17 @@ static void *array_map_lookup_elem(struct bpf_map *map, void *key) ...@@ -117,20 +117,17 @@ static void *array_map_lookup_elem(struct bpf_map *map, void *key)
/* emit BPF instructions equivalent to C code of array_map_lookup_elem() */ /* emit BPF instructions equivalent to C code of array_map_lookup_elem() */
static u32 array_map_gen_lookup(struct bpf_map *map, struct bpf_insn *insn_buf) static u32 array_map_gen_lookup(struct bpf_map *map, struct bpf_insn *insn_buf)
{ {
struct bpf_array *array = container_of(map, struct bpf_array, map);
struct bpf_insn *insn = insn_buf; struct bpf_insn *insn = insn_buf;
u32 elem_size = array->elem_size; u32 elem_size = round_up(map->value_size, 8);
const int ret = BPF_REG_0; const int ret = BPF_REG_0;
const int map_ptr = BPF_REG_1; const int map_ptr = BPF_REG_1;
const int index = BPF_REG_2; const int index = BPF_REG_2;
*insn++ = BPF_ALU64_IMM(BPF_ADD, map_ptr, offsetof(struct bpf_array, value)); *insn++ = BPF_ALU64_IMM(BPF_ADD, map_ptr, offsetof(struct bpf_array, value));
*insn++ = BPF_LDX_MEM(BPF_W, ret, index, 0); *insn++ = BPF_LDX_MEM(BPF_W, ret, index, 0);
*insn++ = BPF_JMP_IMM(BPF_JGE, ret, array->map.max_entries, *insn++ = BPF_JMP_IMM(BPF_JGE, ret, map->max_entries, 3);
elem_size == 1 ? 2 : 3);
if (elem_size == 1) { if (is_power_of_2(elem_size)) {
/* nop */
} else if (is_power_of_2(elem_size)) {
*insn++ = BPF_ALU64_IMM(BPF_LSH, ret, ilog2(elem_size)); *insn++ = BPF_ALU64_IMM(BPF_LSH, ret, ilog2(elem_size));
} else { } else {
*insn++ = BPF_ALU64_IMM(BPF_MUL, ret, elem_size); *insn++ = BPF_ALU64_IMM(BPF_MUL, ret, elem_size);
......
...@@ -143,6 +143,8 @@ struct bpf_verifier_stack_elem { ...@@ -143,6 +143,8 @@ struct bpf_verifier_stack_elem {
#define BPF_COMPLEXITY_LIMIT_INSNS 65536 #define BPF_COMPLEXITY_LIMIT_INSNS 65536
#define BPF_COMPLEXITY_LIMIT_STACK 1024 #define BPF_COMPLEXITY_LIMIT_STACK 1024
#define BPF_MAP_PTR_POISON ((void *)0xeB9F + POISON_POINTER_DELTA)
struct bpf_call_arg_meta { struct bpf_call_arg_meta {
struct bpf_map *map_ptr; struct bpf_map *map_ptr;
bool raw_mode; bool raw_mode;
...@@ -1357,6 +1359,8 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx) ...@@ -1357,6 +1359,8 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx)
} else if (fn->ret_type == RET_VOID) { } else if (fn->ret_type == RET_VOID) {
regs[BPF_REG_0].type = NOT_INIT; regs[BPF_REG_0].type = NOT_INIT;
} else if (fn->ret_type == RET_PTR_TO_MAP_VALUE_OR_NULL) { } else if (fn->ret_type == RET_PTR_TO_MAP_VALUE_OR_NULL) {
struct bpf_insn_aux_data *insn_aux;
regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL; regs[BPF_REG_0].type = PTR_TO_MAP_VALUE_OR_NULL;
regs[BPF_REG_0].max_value = regs[BPF_REG_0].min_value = 0; regs[BPF_REG_0].max_value = regs[BPF_REG_0].min_value = 0;
/* remember map_ptr, so that check_map_access() /* remember map_ptr, so that check_map_access()
...@@ -1369,7 +1373,11 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx) ...@@ -1369,7 +1373,11 @@ static int check_call(struct bpf_verifier_env *env, int func_id, int insn_idx)
} }
regs[BPF_REG_0].map_ptr = meta.map_ptr; regs[BPF_REG_0].map_ptr = meta.map_ptr;
regs[BPF_REG_0].id = ++env->id_gen; regs[BPF_REG_0].id = ++env->id_gen;
env->insn_aux_data[insn_idx].map_ptr = meta.map_ptr; insn_aux = &env->insn_aux_data[insn_idx];
if (!insn_aux->map_ptr)
insn_aux->map_ptr = meta.map_ptr;
else if (insn_aux->map_ptr != meta.map_ptr)
insn_aux->map_ptr = BPF_MAP_PTR_POISON;
} else { } else {
verbose("unknown return type %d of func %s#%d\n", verbose("unknown return type %d of func %s#%d\n",
fn->ret_type, func_id_name(func_id), func_id); fn->ret_type, func_id_name(func_id), func_id);
...@@ -3307,7 +3315,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env) ...@@ -3307,7 +3315,8 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
if (ebpf_jit_enabled() && insn->imm == BPF_FUNC_map_lookup_elem) { if (ebpf_jit_enabled() && insn->imm == BPF_FUNC_map_lookup_elem) {
map_ptr = env->insn_aux_data[i + delta].map_ptr; map_ptr = env->insn_aux_data[i + delta].map_ptr;
if (!map_ptr->ops->map_gen_lookup) if (map_ptr == BPF_MAP_PTR_POISON ||
!map_ptr->ops->map_gen_lookup)
goto patch_call_imm; goto patch_call_imm;
cnt = map_ptr->ops->map_gen_lookup(map_ptr, insn_buf); cnt = map_ptr->ops->map_gen_lookup(map_ptr, insn_buf);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment