Commit 3e8ce298 authored by Alexei Starovoitov's avatar Alexei Starovoitov Committed by Daniel Borkmann

bpf: Prevent pointer mismatch in bpf_timer_init.

bpf_timer_init() arguments are:
1. pointer to a timer (which is embedded in map element).
2. pointer to a map.
Make sure that pointer to a timer actually belongs to that map.

Use map_uid (which is unique id of inner map) to reject:
inner_map1 = bpf_map_lookup_elem(outer_map, key1)
inner_map2 = bpf_map_lookup_elem(outer_map, key2)
if (inner_map1 && inner_map2) {
    timer = bpf_map_lookup_elem(inner_map1);
    if (timer)
        // mismatch would have been allowed
        bpf_timer_init(timer, inner_map2);
}
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Acked-by: default avatarMartin KaFai Lau <kafai@fb.com>
Acked-by: default avatarAndrii Nakryiko <andrii@kernel.org>
Acked-by: default avatarToke Høiland-Jørgensen <toke@redhat.com>
Link: https://lore.kernel.org/bpf/20210715005417.78572-6-alexei.starovoitov@gmail.com
parent 68134668
...@@ -53,7 +53,14 @@ struct bpf_reg_state { ...@@ -53,7 +53,14 @@ struct bpf_reg_state {
/* valid when type == CONST_PTR_TO_MAP | PTR_TO_MAP_VALUE | /* valid when type == CONST_PTR_TO_MAP | PTR_TO_MAP_VALUE |
* PTR_TO_MAP_VALUE_OR_NULL * PTR_TO_MAP_VALUE_OR_NULL
*/ */
struct bpf_map *map_ptr; struct {
struct bpf_map *map_ptr;
/* To distinguish map lookups from outer map
* the map_uid is non-zero for registers
* pointing to inner maps.
*/
u32 map_uid;
};
/* for PTR_TO_BTF_ID */ /* for PTR_TO_BTF_ID */
struct { struct {
......
...@@ -255,6 +255,7 @@ struct bpf_call_arg_meta { ...@@ -255,6 +255,7 @@ struct bpf_call_arg_meta {
int mem_size; int mem_size;
u64 msize_max_value; u64 msize_max_value;
int ref_obj_id; int ref_obj_id;
int map_uid;
int func_id; int func_id;
struct btf *btf; struct btf *btf;
u32 btf_id; u32 btf_id;
...@@ -1135,6 +1136,10 @@ static void mark_ptr_not_null_reg(struct bpf_reg_state *reg) ...@@ -1135,6 +1136,10 @@ static void mark_ptr_not_null_reg(struct bpf_reg_state *reg)
if (map->inner_map_meta) { if (map->inner_map_meta) {
reg->type = CONST_PTR_TO_MAP; reg->type = CONST_PTR_TO_MAP;
reg->map_ptr = map->inner_map_meta; reg->map_ptr = map->inner_map_meta;
/* transfer reg's id which is unique for every map_lookup_elem
* as UID of the inner map.
*/
reg->map_uid = reg->id;
} else if (map->map_type == BPF_MAP_TYPE_XSKMAP) { } else if (map->map_type == BPF_MAP_TYPE_XSKMAP) {
reg->type = PTR_TO_XDP_SOCK; reg->type = PTR_TO_XDP_SOCK;
} else if (map->map_type == BPF_MAP_TYPE_SOCKMAP || } else if (map->map_type == BPF_MAP_TYPE_SOCKMAP ||
...@@ -4708,6 +4713,7 @@ static int process_timer_func(struct bpf_verifier_env *env, int regno, ...@@ -4708,6 +4713,7 @@ static int process_timer_func(struct bpf_verifier_env *env, int regno,
verbose(env, "verifier bug. Two map pointers in a timer helper\n"); verbose(env, "verifier bug. Two map pointers in a timer helper\n");
return -EFAULT; return -EFAULT;
} }
meta->map_uid = reg->map_uid;
meta->map_ptr = map; meta->map_ptr = map;
return 0; return 0;
} }
...@@ -5006,11 +5012,29 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, ...@@ -5006,11 +5012,29 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
if (arg_type == ARG_CONST_MAP_PTR) { if (arg_type == ARG_CONST_MAP_PTR) {
/* bpf_map_xxx(map_ptr) call: remember that map_ptr */ /* bpf_map_xxx(map_ptr) call: remember that map_ptr */
if (meta->map_ptr && meta->map_ptr != reg->map_ptr) { if (meta->map_ptr) {
verbose(env, "Map pointer doesn't match bpf_timer.\n"); /* Use map_uid (which is unique id of inner map) to reject:
return -EINVAL; * inner_map1 = bpf_map_lookup_elem(outer_map, key1)
* inner_map2 = bpf_map_lookup_elem(outer_map, key2)
* if (inner_map1 && inner_map2) {
* timer = bpf_map_lookup_elem(inner_map1);
* if (timer)
* // mismatch would have been allowed
* bpf_timer_init(timer, inner_map2);
* }
*
* Comparing map_ptr is enough to distinguish normal and outer maps.
*/
if (meta->map_ptr != reg->map_ptr ||
meta->map_uid != reg->map_uid) {
verbose(env,
"timer pointer in R1 map_uid=%d doesn't match map pointer in R2 map_uid=%d\n",
meta->map_uid, reg->map_uid);
return -EINVAL;
}
} }
meta->map_ptr = reg->map_ptr; meta->map_ptr = reg->map_ptr;
meta->map_uid = reg->map_uid;
} else if (arg_type == ARG_PTR_TO_MAP_KEY) { } else if (arg_type == ARG_PTR_TO_MAP_KEY) {
/* bpf_map_xxx(..., map_ptr, ..., key) call: /* bpf_map_xxx(..., map_ptr, ..., key) call:
* check that [key, key + map->key_size) are within * check that [key, key + map->key_size) are within
...@@ -6204,6 +6228,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn ...@@ -6204,6 +6228,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
return -EINVAL; return -EINVAL;
} }
regs[BPF_REG_0].map_ptr = meta.map_ptr; regs[BPF_REG_0].map_ptr = meta.map_ptr;
regs[BPF_REG_0].map_uid = meta.map_uid;
if (fn->ret_type == RET_PTR_TO_MAP_VALUE) { if (fn->ret_type == RET_PTR_TO_MAP_VALUE) {
regs[BPF_REG_0].type = PTR_TO_MAP_VALUE; regs[BPF_REG_0].type = PTR_TO_MAP_VALUE;
if (map_value_has_spin_lock(meta.map_ptr)) if (map_value_has_spin_lock(meta.map_ptr))
......
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