Commit 6562e29c authored by John Fastabend's avatar John Fastabend Committed by Daniel Borkmann

bpf: Sockmap, skmsg helper overestimates push, pull, and pop bounds

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: 604326b4 ("bpf, sockmap: convert to generic sk_msg interface")
Fixes: 6fff607e ("bpf: sk_msg program helper bpf_msg_push_data")
Fixes: 7246d8ed ("bpf: helper to pop data from messages")
Signed-off-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Acked-by: default avatarSong Liu <songliubraving@fb.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/bpf/20200111061206.8028-5-john.fastabend@gmail.com
parent 33bfe20d
...@@ -2231,10 +2231,10 @@ BPF_CALL_4(bpf_msg_pull_data, struct sk_msg *, msg, u32, start, ...@@ -2231,10 +2231,10 @@ BPF_CALL_4(bpf_msg_pull_data, struct sk_msg *, msg, u32, start,
/* First find the starting scatterlist element */ /* First find the starting scatterlist element */
i = msg->sg.start; i = msg->sg.start;
do { do {
offset += len;
len = sk_msg_elem(msg, i)->length; len = sk_msg_elem(msg, i)->length;
if (start < offset + len) if (start < offset + len)
break; break;
offset += len;
sk_msg_iter_var_next(i); sk_msg_iter_var_next(i);
} while (i != msg->sg.end); } while (i != msg->sg.end);
...@@ -2346,7 +2346,7 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start, ...@@ -2346,7 +2346,7 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
u32, len, u64, flags) u32, len, u64, flags)
{ {
struct scatterlist sge, nsge, nnsge, rsge = {0}, *psge; 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; u8 *raw, *to, *from;
struct page *page; struct page *page;
...@@ -2356,11 +2356,11 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start, ...@@ -2356,11 +2356,11 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
/* First find the starting scatterlist element */ /* First find the starting scatterlist element */
i = msg->sg.start; i = msg->sg.start;
do { do {
offset += l;
l = sk_msg_elem(msg, i)->length; l = sk_msg_elem(msg, i)->length;
if (start < offset + l) if (start < offset + l)
break; break;
offset += l;
sk_msg_iter_var_next(i); sk_msg_iter_var_next(i);
} while (i != msg->sg.end); } while (i != msg->sg.end);
...@@ -2506,7 +2506,7 @@ static void sk_msg_shift_right(struct sk_msg *msg, int i) ...@@ -2506,7 +2506,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, BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
u32, len, u64, flags) u32, len, u64, flags)
{ {
u32 i = 0, l, space, offset = 0; u32 i = 0, l = 0, space, offset = 0;
u64 last = start + len; u64 last = start + len;
int pop; int pop;
...@@ -2516,11 +2516,11 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start, ...@@ -2516,11 +2516,11 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
/* First find the starting scatterlist element */ /* First find the starting scatterlist element */
i = msg->sg.start; i = msg->sg.start;
do { do {
offset += l;
l = sk_msg_elem(msg, i)->length; l = sk_msg_elem(msg, i)->length;
if (start < offset + l) if (start < offset + l)
break; break;
offset += l;
sk_msg_iter_var_next(i); sk_msg_iter_var_next(i);
} while (i != msg->sg.end); } while (i != msg->sg.end);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment