| From 9bc93e47bb00b7b929a78c41543951a0eebcb302 Mon Sep 17 00:00:00 2001 |
| From: Daniel Borkmann <daniel@iogearbox.net> |
| Date: Mon, 28 Jan 2019 21:23:30 +0100 |
| Subject: bpf: fix inner map masking to prevent oob under speculation |
| |
| [ commit 9d5564ddcf2a0f5ba3fa1c3a1f8a1b59ad309553 upstream ] |
| |
| During review I noticed that inner meta map setup for map in |
| map is buggy in that it does not propagate all needed data |
| from the reference map which the verifier is later accessing. |
| |
| In particular one such case is index masking to prevent out of |
| bounds access under speculative execution due to missing the |
| map's unpriv_array/index_mask field propagation. Fix this such |
| that the verifier is generating the correct code for inlined |
| lookups in case of unpriviledged use. |
| |
| Before patch (test_verifier's 'map in map access' dump): |
| |
| # bpftool prog dump xla id 3 |
| 0: (62) *(u32 *)(r10 -4) = 0 |
| 1: (bf) r2 = r10 |
| 2: (07) r2 += -4 |
| 3: (18) r1 = map[id:4] |
| 5: (07) r1 += 272 | |
| 6: (61) r0 = *(u32 *)(r2 +0) | |
| 7: (35) if r0 >= 0x1 goto pc+6 | Inlined map in map lookup |
| 8: (54) (u32) r0 &= (u32) 0 | with index masking for |
| 9: (67) r0 <<= 3 | map->unpriv_array. |
| 10: (0f) r0 += r1 | |
| 11: (79) r0 = *(u64 *)(r0 +0) | |
| 12: (15) if r0 == 0x0 goto pc+1 | |
| 13: (05) goto pc+1 | |
| 14: (b7) r0 = 0 | |
| 15: (15) if r0 == 0x0 goto pc+11 |
| 16: (62) *(u32 *)(r10 -4) = 0 |
| 17: (bf) r2 = r10 |
| 18: (07) r2 += -4 |
| 19: (bf) r1 = r0 |
| 20: (07) r1 += 272 | |
| 21: (61) r0 = *(u32 *)(r2 +0) | Index masking missing (!) |
| 22: (35) if r0 >= 0x1 goto pc+3 | for inner map despite |
| 23: (67) r0 <<= 3 | map->unpriv_array set. |
| 24: (0f) r0 += r1 | |
| 25: (05) goto pc+1 | |
| 26: (b7) r0 = 0 | |
| 27: (b7) r0 = 0 |
| 28: (95) exit |
| |
| After patch: |
| |
| # bpftool prog dump xla id 1 |
| 0: (62) *(u32 *)(r10 -4) = 0 |
| 1: (bf) r2 = r10 |
| 2: (07) r2 += -4 |
| 3: (18) r1 = map[id:2] |
| 5: (07) r1 += 272 | |
| 6: (61) r0 = *(u32 *)(r2 +0) | |
| 7: (35) if r0 >= 0x1 goto pc+6 | Same inlined map in map lookup |
| 8: (54) (u32) r0 &= (u32) 0 | with index masking due to |
| 9: (67) r0 <<= 3 | map->unpriv_array. |
| 10: (0f) r0 += r1 | |
| 11: (79) r0 = *(u64 *)(r0 +0) | |
| 12: (15) if r0 == 0x0 goto pc+1 | |
| 13: (05) goto pc+1 | |
| 14: (b7) r0 = 0 | |
| 15: (15) if r0 == 0x0 goto pc+12 |
| 16: (62) *(u32 *)(r10 -4) = 0 |
| 17: (bf) r2 = r10 |
| 18: (07) r2 += -4 |
| 19: (bf) r1 = r0 |
| 20: (07) r1 += 272 | |
| 21: (61) r0 = *(u32 *)(r2 +0) | |
| 22: (35) if r0 >= 0x1 goto pc+4 | Now fixed inlined inner map |
| 23: (54) (u32) r0 &= (u32) 0 | lookup with proper index masking |
| 24: (67) r0 <<= 3 | for map->unpriv_array. |
| 25: (0f) r0 += r1 | |
| 26: (05) goto pc+1 | |
| 27: (b7) r0 = 0 | |
| 28: (b7) r0 = 0 |
| 29: (95) exit |
| |
| Fixes: b2157399cc98 ("bpf: prevent out-of-bounds speculation") |
| Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> |
| Acked-by: Martin KaFai Lau <kafai@fb.com> |
| Signed-off-by: Alexei Starovoitov <ast@kernel.org> |
| Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| kernel/bpf/map_in_map.c | 17 +++++++++++++++-- |
| 1 file changed, 15 insertions(+), 2 deletions(-) |
| |
| diff --git a/kernel/bpf/map_in_map.c b/kernel/bpf/map_in_map.c |
| index 99d243e1ad6e..52378d3e34b3 100644 |
| --- a/kernel/bpf/map_in_map.c |
| +++ b/kernel/bpf/map_in_map.c |
| @@ -12,6 +12,7 @@ |
| struct bpf_map *bpf_map_meta_alloc(int inner_map_ufd) |
| { |
| struct bpf_map *inner_map, *inner_map_meta; |
| + u32 inner_map_meta_size; |
| struct fd f; |
| |
| f = fdget(inner_map_ufd); |
| @@ -36,7 +37,12 @@ struct bpf_map *bpf_map_meta_alloc(int inner_map_ufd) |
| return ERR_PTR(-EINVAL); |
| } |
| |
| - inner_map_meta = kzalloc(sizeof(*inner_map_meta), GFP_USER); |
| + inner_map_meta_size = sizeof(*inner_map_meta); |
| + /* In some cases verifier needs to access beyond just base map. */ |
| + if (inner_map->ops == &array_map_ops) |
| + inner_map_meta_size = sizeof(struct bpf_array); |
| + |
| + inner_map_meta = kzalloc(inner_map_meta_size, GFP_USER); |
| if (!inner_map_meta) { |
| fdput(f); |
| return ERR_PTR(-ENOMEM); |
| @@ -46,9 +52,16 @@ struct bpf_map *bpf_map_meta_alloc(int inner_map_ufd) |
| inner_map_meta->key_size = inner_map->key_size; |
| inner_map_meta->value_size = inner_map->value_size; |
| inner_map_meta->map_flags = inner_map->map_flags; |
| - inner_map_meta->ops = inner_map->ops; |
| inner_map_meta->max_entries = inner_map->max_entries; |
| |
| + /* Misc members not needed in bpf_map_meta_equal() check. */ |
| + inner_map_meta->ops = inner_map->ops; |
| + if (inner_map->ops == &array_map_ops) { |
| + inner_map_meta->unpriv_array = inner_map->unpriv_array; |
| + container_of(inner_map_meta, struct bpf_array, map)->index_mask = |
| + container_of(inner_map, struct bpf_array, map)->index_mask; |
| + } |
| + |
| fdput(f); |
| return inner_map_meta; |
| } |
| -- |
| 2.19.1 |
| |