Re: [PATCH bpf] bpf: fix bpf_skb_load_bytes_relative pkt length check

[Alexei Starovoitov]

On Sat, Jul 28, 2018 at 10:04 PM, Daniel Borkmann  wrote:
> The len > skb_headlen(skb) cannot be used as a maximum upper bound
> for the packet length since it does not have any relation to the full
> linear packet length when filtering is used from upper layers (e.g.
> in case of reuseport BPF programs) as by then skb->data, skb->len
> already got mangled through __skb_pull() and others.
>
> Fixes: 4e1ec56cdc59 ("bpf: add skb_load_bytes_relative helper")
> Signed-off-by: Daniel Borkmann 
> Acked-by: Martin KaFai Lau 

Great catch.
Acked-by: Alexei Starovoitov 

[PATCH bpf] bpf: fix bpf_skb_load_bytes_relative pkt length check

[Daniel Borkmann]

The len > skb_headlen(skb) cannot be used as a maximum upper bound
for the packet length since it does not have any relation to the full
linear packet length when filtering is used from upper layers (e.g.
in case of reuseport BPF programs) as by then skb->data, skb->len
already got mangled through __skb_pull() and others.

Fixes: 4e1ec56cdc59 ("bpf: add skb_load_bytes_relative helper")
Signed-off-by: Daniel Borkmann 
Acked-by: Martin KaFai Lau 
---
 net/core/filter.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 06da770..9dfd145 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1712,24 +1712,26 @@ static const struct bpf_func_proto 
bpf_skb_load_bytes_proto = {
 BPF_CALL_5(bpf_skb_load_bytes_relative, const struct sk_buff *, skb,
   u32, offset, void *, to, u32, len, u32, start_header)
 {
+   u8 *end = skb_tail_pointer(skb);
+   u8 *net = skb_network_header(skb);
+   u8 *mac = skb_mac_header(skb);
u8 *ptr;
 
-   if (unlikely(offset > 0x || len > skb_headlen(skb)))
+   if (unlikely(offset > 0x || len > (end - mac)))
goto err_clear;
 
switch (start_header) {
case BPF_HDR_START_MAC:
-   ptr = skb_mac_header(skb) + offset;
+   ptr = mac + offset;
break;
case BPF_HDR_START_NET:
-   ptr = skb_network_header(skb) + offset;
+   ptr = net + offset;
break;
default:
goto err_clear;
}
 
-   if (likely(ptr >= skb_mac_header(skb) &&
-  ptr + len <= skb_tail_pointer(skb))) {
+   if (likely(ptr >= mac && ptr + len <= end)) {
memcpy(to, ptr, len);
return 0;
}
-- 
2.9.5