BPF inline improvements
Subsystems (4)
Daniel Borkmann
maintainerv2 cover BPF inline improvements
First one makes htab inlining more robust wrt future jits and second one inlines map in map lookups through map_gen_lookup() callback.
Thanks!
v1 -> v2:
- BITS_PER_LONG guard in patch 1
- BPF_EMIT_CALL is on __htab_map_lookup_elem
Daniel Borkmann (2):
bpf: make htab inlining more robust wrt assumptions
bpf: inline map in map lookup functions for array and htab
kernel/bpf/arraymap.c | 26 ++++++++++++++++++++++++++ kernel/bpf/hashtab.c | 17 +++++++++++++++++ kernel/bpf/verifier.c | 6 +++++- 3 files changed, 48 insertions(+), 1 deletion(-)
Signature · 1 line
1.9.3
Daniel Borkmann
maintainerPATCH v2 1/2 bpf: make htab inlining more robust wrt assumptions
Commit 9015d2f59535 ("bpf: inline htab_map_lookup_elem()") was making the assumption that a direct call emission to the function __htab_map_lookup_elem() will always work out for JITs.
This is currently true since all JITs we have are for 64 bit archs, but in case of 32 bit JITs like upcoming arm32, we get a NULL pointer dereference when executing the call to __htab_map_lookup_elem() since passed arguments are of a different size (due to pointer args) than what we do out of BPF. Guard and thus limit this for now for the current 64 bit JITs only.
--- kernel/bpf/verifier.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
kernel/bpf/verifier.c +5 -1
@@ -4160,7 +4160,11 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)continue ; } -if (ebpf_jit_enabled () && insn->imm == BPF_FUNC_map_lookup_elem) { +/* BPF_EMIT_CALL() assumptions in some of the map_gen_lookup +* handlers are currently limited to 64 bit only. +*/ +if (ebpf_jit_enabled () &&BITS_PER_LONG ==64 && + insn->imm == BPF_FUNC_map_lookup_elem) { map_ptr = env->insn_aux_data[i + delta].map_ptr;if (map_ptr ==BPF_MAP_PTR_POISON || !map_ptr->ops->map_gen_lookup)
Signature · 1 line
1.9.3
Re: [PATCH net-next v2 1/2] bpf: make htab inlining more robust wrt assumptions
Quoted 11 lines · Daniel Borkmann
Commit
9015d2f59535("bpf: inlinehtab_map_lookup_elem()") was making the assumption that a direct call emission to the function__htab_map_lookup_elem()will always work out for JITs.This is currently true since all JITs we have are for 64 bit archs, but in case of 32 bit JITs like upcoming arm32, we get a NULL pointer dereference when executing the call to
__htab_map_lookup_elem()since passed arguments are of a different size (due to pointer args) than what we do out of BPF. Guard and thus limit this for now for the current 64 bit JITs only.
Quoted 2 lines
Reported-by Shubham Bansal <[email protected]>Signed-off-by Daniel Borkmann <[email protected]>
Thanks. That's good robustness fix.
Daniel Borkmann
maintainerPATCH v2 2/2 bpf: inline map in map lookup functions for array and htab
In reply to Daniel BorkmannAvoid two successive functions calls for the map in map lookup, first is the bpf_map_lookup_elem() helper call, and second the callback via map->ops->map_lookup_elem() to get to the map in map implementation. Implementation inlines array and htab flavor for map in map lookups.
--- kernel/bpf/arraymap.c | 26 ++++++++++++++++++++++++++ kernel/bpf/hashtab.c | 17 +++++++++++++++++ 2 files changed, 43 insertions(+)
kernel/bpf/arraymap.c +26 -0
@@ -603,6 +603,31 @@ static void *array_of_map_lookup_elem(struct bpf_map *map, void *key)return READ_ONCE (*inner_map); } +static u32 array_of_map_gen_lookup (struct bpf_map *map, +struct bpf_insn *insn_buf) +{ +u32 elem_size =round_up (map->value_size,8 ); +struct bpf_insn *insn = insn_buf; +const int ret =BPF_REG_0 ; +const int map_ptr =BPF_REG_1 ; +const int index =BPF_REG_2 ; + + *insn++ =BPF_ALU64_IMM (BPF_ADD , map_ptr, offsetof(struct bpf_array , value)); + *insn++ =BPF_LDX_MEM (BPF_W , ret, index,0 ); + *insn++ =BPF_JMP_IMM (BPF_JGE , ret, map->max_entries,5 ); +if (is_power_of_2 (elem_size)) + *insn++ =BPF_ALU64_IMM (BPF_LSH , ret,ilog2 (elem_size)); +else + *insn++ =BPF_ALU64_IMM (BPF_MUL , ret, elem_size); + *insn++ =BPF_ALU64_REG (BPF_ADD , ret, map_ptr); + *insn++ =BPF_LDX_MEM (BPF_DW , ret, ret,0 ); + *insn++ =BPF_JMP_IMM (BPF_JEQ , ret,0 ,1 ); + *insn++ =BPF_JMP_IMM (BPF_JA ,0 ,0 ,1 ); + *insn++ =BPF_MOV64_IMM (ret,0 ); + +return insn - insn_buf; +} +const struct bpf_map_ops array_of_maps_map_ops = { .map_alloc = array_of_map_alloc, .map_free = array_of_map_free,
@@ -612,4 +637,5 @@ static void *array_of_map_lookup_elem(struct bpf_map *map, void *key) .map_fd_get_ptr = bpf_map_fd_get_ptr, .map_fd_put_ptr = bpf_map_fd_put_ptr, .map_fd_sys_lookup_elem = bpf_map_fd_sys_lookup_elem, + .map_gen_lookup = array_of_map_gen_lookup, };
kernel/bpf/hashtab.c +17 -0
@@ -1311,6 +1311,22 @@ static void *htab_of_map_lookup_elem(struct bpf_map *map, void *key)return READ_ONCE (*inner_map); } +static u32 htab_of_map_gen_lookup (struct bpf_map *map, +struct bpf_insn *insn_buf) +{ +struct bpf_insn *insn = insn_buf; +const int ret =BPF_REG_0 ; + + *insn++ =BPF_EMIT_CALL ((u64 (*)(u64 ,u64 ,u64 ,u64 ,u64 ))__htab_map_lookup_elem); + *insn++ =BPF_JMP_IMM (BPF_JEQ , ret,0 ,2 ); + *insn++ =BPF_ALU64_IMM (BPF_ADD , ret, + offsetof(struct htab_elem , key) + +round_up (map->key_size,8 )); + *insn++ =BPF_LDX_MEM (BPF_DW , ret, ret,0 ); + +return insn - insn_buf; +} +static void htab_of_map_free (struct bpf_map *map) {bpf_map_meta_free (map->inner_map_meta);
@@ -1326,4 +1342,5 @@ static void htab_of_map_free(struct bpf_map *map) .map_fd_get_ptr = bpf_map_fd_get_ptr, .map_fd_put_ptr = bpf_map_fd_put_ptr, .map_fd_sys_lookup_elem = bpf_map_fd_sys_lookup_elem, + .map_gen_lookup = htab_of_map_gen_lookup, };
Signature · 1 line
1.9.3
David Miller
maintainerFrom: Daniel Borkmann <[email protected]>
Date: Sat, 19 Aug 2017 03:12:44 +0200
Quoted 3 lines
First one makes htab inlining more robust wrt future jits and second one inlines map in map lookups through
map_gen_lookup()callback.
Series applied, thanks Daniel.