On Mon, Dec 3, 2018 at 8:01 PM Stanislav Fomichev <s...@google.com> wrote:
>
> We want to make sure that the following condition holds:
> 0 <= nhoff <= thoff <= skb->len
>
> BPF program can set out-of-bounds nhoff and thoff, which is dangerous, see
> recent commit d0c081b49137 ("flow_dissector: properly cap thoff field")'.
>
> Signed-off-by: Stanislav Fomichev <s...@google.com>
> ---
> net/core/flow_dissector.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index ac19da6f390b..bb1a54747d64 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -716,6 +716,10 @@ bool __skb_flow_bpf_dissect(struct bpf_prog *prog,
> /* Restore state */
> memcpy(cb, &cb_saved, sizeof(cb_saved));
>
> + flow_keys->nhoff = clamp_t(u16, flow_keys->nhoff, 0, skb->len);
> + flow_keys->thoff = clamp_t(u16, flow_keys->thoff,
> + flow_keys->nhoff, skb->len);
> +
> return result == BPF_OK;
> }
>
> @@ -808,8 +812,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
> &flow_keys);
> __skb_flow_bpf_to_target(&flow_keys, flow_dissector,
> target_container);
> - key_control->thoff = min_t(u16, key_control->thoff,
> - skb->len);
> rcu_read_unlock();
> return ret;
> }
> --
> 2.20.0.rc1.387.gf8505762e3-goog
>
Same question as 3/5:
Do we need this fix without this set? If yes, do we need it for bpf
tree as well?
Thanks,
Song
