Skip to content

Commit

Permalink
bpf: fix shift upon scatterlist ring wrap-around in bpf_msg_pull_data
Browse files Browse the repository at this point in the history
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: 015632b ("bpf: sk_msg program helper bpf_sk_msg_pull_data")
Signed-off-by: Daniel Borkmann <[email protected]>
Acked-by: John Fastabend <[email protected]>
Signed-off-by: Alexei Starovoitov <[email protected]>
  • Loading branch information
borkmann authored and Alexei Starovoitov committed Aug 29, 2018
1 parent 0e06b22 commit 2e43f95
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion net/core/filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -2370,7 +2370,10 @@ BPF_CALL_4(bpf_msg_pull_data,
* had a single entry though we can just replace it and
* be done. Otherwise walk the ring and shift the entries.
*/
shift = last_sg - first_sg - 1;
WARN_ON_ONCE(last_sg == first_sg);
shift = last_sg > first_sg ?
last_sg - first_sg - 1 :
MAX_SKB_FRAGS - first_sg + last_sg - 1;
if (!shift)
goto out;

Expand Down

0 comments on commit 2e43f95

Please sign in to comment.