| From d36392de9c611e59b56e6b343c39086570bb1c88 Mon Sep 17 00:00:00 2001 |
| From: John Fastabend <john.fastabend@gmail.com> |
| Date: Sat, 11 Jan 2020 06:12:02 +0000 |
| Subject: [PATCH] bpf: Sockmap, skmsg helper overestimates push, pull, and pop |
| bounds |
| |
| commit 6562e29cf6f0ddd368657d97a8d484ffc30df5ef upstream. |
| |
| In the push, pull, and pop helpers operating on skmsg objects to make |
| data writable or insert/remove data we use this bounds check to ensure |
| specified data is valid, |
| |
| /* Bounds checks: start and pop must be inside message */ |
| if (start >= offset + l || last >= msg->sg.size) |
| return -EINVAL; |
| |
| The problem here is offset has already included the length of the |
| current element the 'l' above. So start could be past the end of |
| the scatterlist element in the case where start also points into an |
| offset on the last skmsg element. |
| |
| To fix do the accounting slightly different by adding the length of |
| the previous entry to offset at the start of the iteration. And |
| ensure its initialized to zero so that the first iteration does |
| nothing. |
| |
| Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface") |
| Fixes: 6fff607e2f14b ("bpf: sk_msg program helper bpf_msg_push_data") |
| Fixes: 7246d8ed4dcce ("bpf: helper to pop data from messages") |
| Signed-off-by: John Fastabend <john.fastabend@gmail.com> |
| Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> |
| Acked-by: Song Liu <songliubraving@fb.com> |
| Cc: stable@vger.kernel.org |
| Link: https://lore.kernel.org/bpf/20200111061206.8028-5-john.fastabend@gmail.com |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/net/core/filter.c b/net/core/filter.c |
| index 7acc2bd95f39..1b78e0646279 100644 |
| --- a/net/core/filter.c |
| +++ b/net/core/filter.c |
| @@ -2230,10 +2230,10 @@ BPF_CALL_4(bpf_msg_pull_data, struct sk_msg *, msg, u32, start, |
| /* First find the starting scatterlist element */ |
| i = msg->sg.start; |
| do { |
| + offset += len; |
| len = sk_msg_elem(msg, i)->length; |
| if (start < offset + len) |
| break; |
| - offset += len; |
| sk_msg_iter_var_next(i); |
| } while (i != msg->sg.end); |
| |
| @@ -2345,7 +2345,7 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start, |
| u32, len, u64, flags) |
| { |
| struct scatterlist sge, nsge, nnsge, rsge = {0}, *psge; |
| - u32 new, i = 0, l, space, copy = 0, offset = 0; |
| + u32 new, i = 0, l = 0, space, copy = 0, offset = 0; |
| u8 *raw, *to, *from; |
| struct page *page; |
| |
| @@ -2355,11 +2355,11 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start, |
| /* First find the starting scatterlist element */ |
| i = msg->sg.start; |
| do { |
| + offset += l; |
| l = sk_msg_elem(msg, i)->length; |
| |
| if (start < offset + l) |
| break; |
| - offset += l; |
| sk_msg_iter_var_next(i); |
| } while (i != msg->sg.end); |
| |
| @@ -2505,7 +2505,7 @@ static void sk_msg_shift_right(struct sk_msg *msg, int i) |
| BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start, |
| u32, len, u64, flags) |
| { |
| - u32 i = 0, l, space, offset = 0; |
| + u32 i = 0, l = 0, space, offset = 0; |
| u64 last = start + len; |
| int pop; |
| |
| @@ -2515,11 +2515,11 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start, |
| /* First find the starting scatterlist element */ |
| i = msg->sg.start; |
| do { |
| + offset += l; |
| l = sk_msg_elem(msg, i)->length; |
| |
| if (start < offset + l) |
| break; |
| - offset += l; |
| sk_msg_iter_var_next(i); |
| } while (i != msg->sg.end); |
| |
| -- |
| 2.7.4 |
| |