* [PATCH bpf] bpf: fix inner map masking to prevent oob under speculation
@ 2019-01-17 15:34 Daniel Borkmann
2019-01-17 18:32 ` Martin Lau
2019-01-18 23:25 ` Alexei Starovoitov
0 siblings, 2 replies; 4+ messages in thread
From: Daniel Borkmann @ 2019-01-17 15:34 UTC (permalink / raw)
To: ast; +Cc: netdev, Daniel Borkmann
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>
---
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 99d243e..52378d3 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.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf] bpf: fix inner map masking to prevent oob under speculation
2019-01-17 15:34 [PATCH bpf] bpf: fix inner map masking to prevent oob under speculation Daniel Borkmann
@ 2019-01-17 18:32 ` Martin Lau
2019-01-17 23:10 ` Daniel Borkmann
2019-01-18 23:25 ` Alexei Starovoitov
1 sibling, 1 reply; 4+ messages in thread
From: Martin Lau @ 2019-01-17 18:32 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast@kernel.org, netdev@vger.kernel.org
On Thu, Jan 17, 2019 at 04:34:45PM +0100, Daniel Borkmann wrote:
> 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>
The fix looks great. Thanks for the fix!
In the future if there is another exception other than
array_map, a inner_map->ops->map_meta_alloc() can be introduced?
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf] bpf: fix inner map masking to prevent oob under speculation
2019-01-17 18:32 ` Martin Lau
@ 2019-01-17 23:10 ` Daniel Borkmann
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Borkmann @ 2019-01-17 23:10 UTC (permalink / raw)
To: Martin Lau; +Cc: ast@kernel.org, netdev@vger.kernel.org
On 01/17/2019 07:32 PM, Martin Lau wrote:
> On Thu, Jan 17, 2019 at 04:34:45PM +0100, Daniel Borkmann wrote:
>> 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>
> The fix looks great. Thanks for the fix!
> In the future if there is another exception other than
> array_map, a inner_map->ops->map_meta_alloc() can be introduced?
Yeah, probably makes sense if there will be more users in future.
> Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf] bpf: fix inner map masking to prevent oob under speculation
2019-01-17 15:34 [PATCH bpf] bpf: fix inner map masking to prevent oob under speculation Daniel Borkmann
2019-01-17 18:32 ` Martin Lau
@ 2019-01-18 23:25 ` Alexei Starovoitov
1 sibling, 0 replies; 4+ messages in thread
From: Alexei Starovoitov @ 2019-01-18 23:25 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast, netdev
On Thu, Jan 17, 2019 at 04:34:45PM +0100, Daniel Borkmann wrote:
> 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.
>
>
> Fixes: b2157399cc98 ("bpf: prevent out-of-bounds speculation")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Applied, Thanks
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-01-18 23:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-17 15:34 [PATCH bpf] bpf: fix inner map masking to prevent oob under speculation Daniel Borkmann
2019-01-17 18:32 ` Martin Lau
2019-01-17 23:10 ` Daniel Borkmann
2019-01-18 23:25 ` Alexei Starovoitov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).