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: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
Link: https://lore.kernel.org/bpf/20210715005417.78572-6-alexei.starovoitov@gmail.com

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index e774ecc1cd1f517baf237bca43f09c5df4828fdb..5d3169b57e6e2d5d8afc1493eb259a31d30c7d06 100644 (file)
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -53,7 +53,14 @@ struct bpf_reg_state {
                /* valid when type == CONST_PTR_TO_MAP | PTR_TO_MAP_VALUE |
                 *   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 */
                struct {

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 12b50f46a7c196b398ad0c2af554cff8acc24d17..8df2671c3d33dbea5e38a0b9e8c129dd7d18d6c7 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -255,6 +255,7 @@ struct bpf_call_arg_meta {
        int mem_size;
        u64 msize_max_value;
        int ref_obj_id;
+       int map_uid;
        int func_id;
        struct btf *btf;
        u32 btf_id;
@@ -1135,6 +1136,10 @@ static void mark_ptr_not_null_reg(struct bpf_reg_state *reg)
                if (map->inner_map_meta) {
                        reg->type = CONST_PTR_TO_MAP;
                        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) {
                        reg->type = PTR_TO_XDP_SOCK;
                } 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,
                verbose(env, "verifier bug. Two map pointers in a timer helper\n");
                return -EFAULT;
        }
+       meta->map_uid = reg->map_uid;
        meta->map_ptr = map;
        return 0;
 }
@@ -5006,11 +5012,29 @@ skip_type_check:
 
        if (arg_type == ARG_CONST_MAP_PTR) {
                /* bpf_map_xxx(map_ptr) call: remember that map_ptr */
-               if (meta->map_ptr && meta->map_ptr != reg->map_ptr) {
-                       verbose(env, "Map pointer doesn't match bpf_timer.\n");
-                       return -EINVAL;
+               if (meta->map_ptr) {
+                       /* 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);
+                        * }
+                        *
+                        * 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_uid = reg->map_uid;
        } else if (arg_type == ARG_PTR_TO_MAP_KEY) {
                /* bpf_map_xxx(..., map_ptr, ..., key) call:
                 * 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
                        return -EINVAL;
                }
                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) {
                        regs[BPF_REG_0].type = PTR_TO_MAP_VALUE;
                        if (map_value_has_spin_lock(meta.map_ptr))