diff options
author | Daniel Borkmann <daniel@iogearbox.net> | 2018-08-29 10:50:35 -0400 |
---|---|---|
committer | Alexei Starovoitov <ast@kernel.org> | 2018-08-29 13:47:17 -0400 |
commit | 2e43f95dd8ee62bc8bf57f2afac37fbd70c8d565 (patch) | |
tree | e63a37df4d26585edc7061b480a256f5e6e5ab91 | |
parent | 0e06b227c5221dd51b5569de93f3b9f532be4a32 (diff) |
bpf: fix shift upon scatterlist ring wrap-around in bpf_msg_pull_data
If first_sg and last_sg wraps around in the scatterlist ring, then we
need to account for that in the shift as well. E.g. crafting such msgs
where this is the case leads to a hang as shift becomes negative. E.g.
consider the following scenario:
first_sg := 14 |=> shift := -12 msg->sg_start := 10
last_sg := 3 | msg->sg_end := 5
round 1: i := 15, move_from := 3, sg[15] := sg[ 3]
round 2: i := 0, move_from := -12, sg[ 0] := sg[-12]
round 3: i := 1, move_from := -11, sg[ 1] := sg[-11]
round 4: i := 2, move_from := -10, sg[ 2] := sg[-10]
[...]
round 13: i := 11, move_from := -1, sg[ 2] := sg[ -1]
round 14: i := 12, move_from := 0, sg[ 2] := sg[ 0]
round 15: i := 13, move_from := 1, sg[ 2] := sg[ 1]
round 16: i := 14, move_from := 2, sg[ 2] := sg[ 2]
round 17: i := 15, move_from := 3, sg[ 2] := sg[ 3]
[...]
This means we will loop forever and never hit the msg->sg_end condition
to break out of the loop. When we see that the ring wraps around, then
the shift should be MAX_SKB_FRAGS - first_sg + last_sg - 1. Meaning,
the remainder slots from the tail of the ring and the head until last_sg
combined.
Fixes: 015632bb30da ("bpf: sk_msg program helper bpf_sk_msg_pull_data")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
-rw-r--r-- | net/core/filter.c | 5 |
1 files changed, 4 insertions, 1 deletions
diff --git a/net/core/filter.c b/net/core/filter.c index b9225c55926a..43ba5f8c38ca 100644 --- a/net/core/filter.c +++ b/net/core/filter.c | |||
@@ -2370,7 +2370,10 @@ BPF_CALL_4(bpf_msg_pull_data, | |||
2370 | * had a single entry though we can just replace it and | 2370 | * had a single entry though we can just replace it and |
2371 | * be done. Otherwise walk the ring and shift the entries. | 2371 | * be done. Otherwise walk the ring and shift the entries. |
2372 | */ | 2372 | */ |
2373 | shift = last_sg - first_sg - 1; | 2373 | WARN_ON_ONCE(last_sg == first_sg); |
2374 | shift = last_sg > first_sg ? | ||
2375 | last_sg - first_sg - 1 : | ||
2376 | MAX_SKB_FRAGS - first_sg + last_sg - 1; | ||
2374 | if (!shift) | 2377 | if (!shift) |
2375 | goto out; | 2378 | goto out; |
2376 | 2379 | ||