| From d84552ed132315a2a09c28447f152e8239758b1f Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Tue, 8 Sep 2020 10:57:02 -0700 |
| Subject: bpf: Permit map_ptr arithmetic with opcode add and offset 0 |
| |
| From: Yonghong Song <yhs@fb.com> |
| |
| [ Upstream commit 7c6967326267bd5c0dded0a99541357d70dd11ac ] |
| |
| Commit 41c48f3a98231 ("bpf: Support access |
| to bpf map fields") added support to access map fields |
| with CORE support. For example, |
| |
| struct bpf_map { |
| __u32 max_entries; |
| } __attribute__((preserve_access_index)); |
| |
| struct bpf_array { |
| struct bpf_map map; |
| __u32 elem_size; |
| } __attribute__((preserve_access_index)); |
| |
| struct { |
| __uint(type, BPF_MAP_TYPE_ARRAY); |
| __uint(max_entries, 4); |
| __type(key, __u32); |
| __type(value, __u32); |
| } m_array SEC(".maps"); |
| |
| SEC("cgroup_skb/egress") |
| int cg_skb(void *ctx) |
| { |
| struct bpf_array *array = (struct bpf_array *)&m_array; |
| |
| /* .. array->map.max_entries .. */ |
| } |
| |
| In kernel, bpf_htab has similar structure, |
| |
| struct bpf_htab { |
| struct bpf_map map; |
| ... |
| } |
| |
| In the above cg_skb(), to access array->map.max_entries, with CORE, the clang will |
| generate two builtin's. |
| base = &m_array; |
| /* access array.map */ |
| map_addr = __builtin_preserve_struct_access_info(base, 0, 0); |
| /* access array.map.max_entries */ |
| max_entries_addr = __builtin_preserve_struct_access_info(map_addr, 0, 0); |
| max_entries = *max_entries_addr; |
| |
| In the current llvm, if two builtin's are in the same function or |
| in the same function after inlining, the compiler is smart enough to chain |
| them together and generates like below: |
| base = &m_array; |
| max_entries = *(base + reloc_offset); /* reloc_offset = 0 in this case */ |
| and we are fine. |
| |
| But if we force no inlining for one of functions in test_map_ptr() selftest, e.g., |
| check_default(), the above two __builtin_preserve_* will be in two different |
| functions. In this case, we will have code like: |
| func check_hash(): |
| reloc_offset_map = 0; |
| base = &m_array; |
| map_base = base + reloc_offset_map; |
| check_default(map_base, ...) |
| func check_default(map_base, ...): |
| max_entries = *(map_base + reloc_offset_max_entries); |
| |
| In kernel, map_ptr (CONST_PTR_TO_MAP) does not allow any arithmetic. |
| The above "map_base = base + reloc_offset_map" will trigger a verifier failure. |
| ; VERIFY(check_default(&hash->map, map)); |
| 0: (18) r7 = 0xffffb4fe8018a004 |
| 2: (b4) w1 = 110 |
| 3: (63) *(u32 *)(r7 +0) = r1 |
| R1_w=invP110 R7_w=map_value(id=0,off=4,ks=4,vs=8,imm=0) R10=fp0 |
| ; VERIFY_TYPE(BPF_MAP_TYPE_HASH, check_hash); |
| 4: (18) r1 = 0xffffb4fe8018a000 |
| 6: (b4) w2 = 1 |
| 7: (63) *(u32 *)(r1 +0) = r2 |
| R1_w=map_value(id=0,off=0,ks=4,vs=8,imm=0) R2_w=invP1 R7_w=map_value(id=0,off=4,ks=4,vs=8,imm=0) R10=fp0 |
| 8: (b7) r2 = 0 |
| 9: (18) r8 = 0xffff90bcb500c000 |
| 11: (18) r1 = 0xffff90bcb500c000 |
| 13: (0f) r1 += r2 |
| R1 pointer arithmetic on map_ptr prohibited |
| |
| To fix the issue, let us permit map_ptr + 0 arithmetic which will |
| result in exactly the same map_ptr. |
| |
| Signed-off-by: Yonghong Song <yhs@fb.com> |
| Signed-off-by: Alexei Starovoitov <ast@kernel.org> |
| Acked-by: Andrii Nakryiko <andriin@fb.com> |
| Link: https://lore.kernel.org/bpf/20200908175702.2463625-1-yhs@fb.com |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| kernel/bpf/verifier.c | 4 ++++ |
| 1 file changed, 4 insertions(+) |
| |
| diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c |
| index 43cd175c66a55..718bbdc8b3c66 100644 |
| --- a/kernel/bpf/verifier.c |
| +++ b/kernel/bpf/verifier.c |
| @@ -5246,6 +5246,10 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, |
| dst, reg_type_str[ptr_reg->type]); |
| return -EACCES; |
| case CONST_PTR_TO_MAP: |
| + /* smin_val represents the known value */ |
| + if (known && smin_val == 0 && opcode == BPF_ADD) |
| + break; |
| + /* fall-through */ |
| case PTR_TO_PACKET_END: |
| case PTR_TO_SOCKET: |
| case PTR_TO_SOCKET_OR_NULL: |
| -- |
| 2.27.0 |
| |