lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPhsuW6AaStJDqUgEO+-r=LojA1DTbCYR=smM89qxhVCVJ-7Kw@mail.gmail.com>
Date:   Fri, 25 Jan 2019 16:29:56 -0800
From:   Song Liu <liu.song.a23@...il.com>
To:     Stanislav Fomichev <sdf@...gle.com>
Cc:     Networking <netdev@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [PATCH bpf-next v2 1/3] net/flow_dissector: move bpf case into __skb_flow_bpf_dissect

On Thu, Jan 24, 2019 at 8:51 AM Stanislav Fomichev <sdf@...gle.com> wrote:
>
> This way, we can reuse it for flow dissector in BPF_PROG_TEST_RUN.
>
> No functional changes.
>
> Signed-off-by: Stanislav Fomichev <sdf@...gle.com>

Acked-by: Song Liu <songliubraving@...com>

> ---
>  include/linux/skbuff.h    |  5 +++
>  net/core/flow_dissector.c | 92 +++++++++++++++++++++++----------------
>  2 files changed, 59 insertions(+), 38 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 93f56fddd92a..be762fc34ff3 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -1221,6 +1221,11 @@ static inline int skb_flow_dissector_bpf_prog_detach(const union bpf_attr *attr)
>  }
>  #endif
>
> +struct bpf_flow_keys;
> +bool __skb_flow_bpf_dissect(struct bpf_prog *prog,
> +                           const struct sk_buff *skb,
> +                           struct flow_dissector *flow_dissector,
> +                           struct bpf_flow_keys *flow_keys);
>  bool __skb_flow_dissect(const struct sk_buff *skb,
>                         struct flow_dissector *flow_dissector,
>                         void *target_container,
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 9f2840510e63..bb1a54747d64 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -683,6 +683,46 @@ static void __skb_flow_bpf_to_target(const struct bpf_flow_keys *flow_keys,
>         }
>  }
>
> +bool __skb_flow_bpf_dissect(struct bpf_prog *prog,
> +                           const struct sk_buff *skb,
> +                           struct flow_dissector *flow_dissector,
> +                           struct bpf_flow_keys *flow_keys)
> +{
> +       struct bpf_skb_data_end cb_saved;
> +       struct bpf_skb_data_end *cb;
> +       u32 result;
> +
> +       /* Note that even though the const qualifier is discarded
> +        * throughout the execution of the BPF program, all changes(the
> +        * control block) are reverted after the BPF program returns.
> +        * Therefore, __skb_flow_dissect does not alter the skb.
> +        */
> +
> +       cb = (struct bpf_skb_data_end *)skb->cb;
> +
> +       /* Save Control Block */
> +       memcpy(&cb_saved, cb, sizeof(cb_saved));
> +       memset(cb, 0, sizeof(*cb));
> +
> +       /* Pass parameters to the BPF program */
> +       memset(flow_keys, 0, sizeof(*flow_keys));
> +       cb->qdisc_cb.flow_keys = flow_keys;
> +       flow_keys->nhoff = skb_network_offset(skb);
> +       flow_keys->thoff = flow_keys->nhoff;
> +
> +       bpf_compute_data_pointers((struct sk_buff *)skb);
> +       result = BPF_PROG_RUN(prog, skb);
> +
> +       /* 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;
> +}
> +
>  /**
>   * __skb_flow_dissect - extract the flow_keys struct and return it
>   * @skb: sk_buff to extract the flow from, can be NULL if the rest are specified
> @@ -714,7 +754,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>         struct flow_dissector_key_vlan *key_vlan;
>         enum flow_dissect_ret fdret;
>         enum flow_dissector_key_id dissector_vlan = FLOW_DISSECTOR_KEY_MAX;
> -       struct bpf_prog *attached = NULL;
>         int num_hdrs = 0;
>         u8 ip_proto = 0;
>         bool ret;
> @@ -754,53 +793,30 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>                                               FLOW_DISSECTOR_KEY_BASIC,
>                                               target_container);
>
> -       rcu_read_lock();
>         if (skb) {
> +               struct bpf_flow_keys flow_keys;
> +               struct bpf_prog *attached = NULL;
> +
> +               rcu_read_lock();
> +
>                 if (skb->dev)
>                         attached = rcu_dereference(dev_net(skb->dev)->flow_dissector_prog);
>                 else if (skb->sk)
>                         attached = rcu_dereference(sock_net(skb->sk)->flow_dissector_prog);
>                 else
>                         WARN_ON_ONCE(1);
> -       }
> -       if (attached) {
> -               /* Note that even though the const qualifier is discarded
> -                * throughout the execution of the BPF program, all changes(the
> -                * control block) are reverted after the BPF program returns.
> -                * Therefore, __skb_flow_dissect does not alter the skb.
> -                */
> -               struct bpf_flow_keys flow_keys = {};
> -               struct bpf_skb_data_end cb_saved;
> -               struct bpf_skb_data_end *cb;
> -               u32 result;
> -
> -               cb = (struct bpf_skb_data_end *)skb->cb;
> -
> -               /* Save Control Block */
> -               memcpy(&cb_saved, cb, sizeof(cb_saved));
> -               memset(cb, 0, sizeof(cb_saved));
>
> -               /* Pass parameters to the BPF program */
> -               cb->qdisc_cb.flow_keys = &flow_keys;
> -               flow_keys.nhoff = nhoff;
> -               flow_keys.thoff = nhoff;
> -
> -               bpf_compute_data_pointers((struct sk_buff *)skb);
> -               result = BPF_PROG_RUN(attached, skb);
> -
> -               /* 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);
> -
> -               __skb_flow_bpf_to_target(&flow_keys, flow_dissector,
> -                                        target_container);
> +               if (attached) {
> +                       ret = __skb_flow_bpf_dissect(attached, skb,
> +                                                    flow_dissector,
> +                                                    &flow_keys);
> +                       __skb_flow_bpf_to_target(&flow_keys, flow_dissector,
> +                                                target_container);
> +                       rcu_read_unlock();
> +                       return ret;
> +               }
>                 rcu_read_unlock();
> -               return result == BPF_OK;
>         }
> -       rcu_read_unlock();
>
>         if (dissector_uses_key(flow_dissector,
>                                FLOW_DISSECTOR_KEY_ETH_ADDRS)) {
> --
> 2.20.1.321.g9e740568ce-goog
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ