On Tue, Jul 24, 2018 at 04:59:54PM +0000, Mathieu Xhonneux wrote:
> The seg6local LWT provides the End.DT6 action, which allows to
> decapsulate an outer IPv6 header containing a Segment Routing Header
> (SRH), full specification is available here:
> 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dfilsfils-2Dspring-2Dsrv6-2Dnetwork-2Dprogramming-2D05&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=VQnoQ7LvghIj0gVEaiQSUw&m=c61PGnhPMmCUcL5lpyBsxOmsBU2mU5KFY0-Ioo-pBC4&s=mzShtRc5ofzfknAuqoehbGN1ifA17aKihiVLJVfkuZ8&e=
> 
> This patch adds this action now to the seg6local BPF
> interface. Since it is not mandatory that the inner IPv6 header also
> contains a SRH, seg6_bpf_srh_state has been extended with a pointer to
> a possible SRH of the outermost IPv6 header. This helps assessing if the
> validation must be triggered or not, and avoids some calls to
> ipv6_find_hdr.
> 
> Signed-off-by: Mathieu Xhonneux <m.xhonn...@gmail.com>
> ---
>  include/net/seg6_local.h |  4 ++-
>  net/core/filter.c        | 83 
> +++++++++++++++++++++++++++++++++---------------
>  net/ipv6/seg6_local.c    | 42 +++++++++++++++---------
>  3 files changed, 87 insertions(+), 42 deletions(-)
> 
> diff --git a/include/net/seg6_local.h b/include/net/seg6_local.h
> index 661fd5b4d3e0..08359e2d8b35 100644
> --- a/include/net/seg6_local.h
> +++ b/include/net/seg6_local.h
> @@ -21,10 +21,12 @@
>  
>  extern int seg6_lookup_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr,
>                              u32 tbl_id);
> +extern bool seg6_bpf_has_valid_srh(struct sk_buff *skb);
>  
>  struct seg6_bpf_srh_state {
> -     bool valid;
> +     struct ipv6_sr_hdr *srh;
>       u16 hdrlen;
> +     bool valid;
>  };
>  
>  DECLARE_PER_CPU(struct seg6_bpf_srh_state, seg6_bpf_srh_states);
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 104d560946da..2cdea7d05063 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4542,14 +4542,13 @@ BPF_CALL_4(bpf_lwt_seg6_store_bytes, struct sk_buff 
> *, skb, u32, offset,
>  {
>       struct seg6_bpf_srh_state *srh_state =
>               this_cpu_ptr(&seg6_bpf_srh_states);
> +     struct ipv6_sr_hdr *srh = srh_state->srh;
>       void *srh_tlvs, *srh_end, *ptr;
> -     struct ipv6_sr_hdr *srh;
>       int srhoff = 0;
>  
> -     if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
> +     if (srh == NULL)
>               return -EINVAL;
>  
> -     srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
>       srh_tlvs = (void *)((char *)srh + ((srh->first_segment + 1) << 4));
>       srh_end = (void *)((char *)srh + sizeof(*srh) + srh_state->hdrlen);
>  
> @@ -4562,6 +4561,9 @@ BPF_CALL_4(bpf_lwt_seg6_store_bytes, struct sk_buff *, 
> skb, u32, offset,
>  
>       if (unlikely(bpf_try_make_writable(skb, offset + len)))
>               return -EFAULT;
> +     if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
> +             return -EINVAL;
> +     srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
>  
>       memcpy(skb->data + offset, from, len);
>       return 0;
> @@ -4577,52 +4579,79 @@ static const struct bpf_func_proto 
> bpf_lwt_seg6_store_bytes_proto = {
>       .arg4_type      = ARG_CONST_SIZE
>  };
>  
> -BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
> -        u32, action, void *, param, u32, param_len)
> +static void bpf_update_srh_state(struct sk_buff *skb)
>  {
>       struct seg6_bpf_srh_state *srh_state =
>               this_cpu_ptr(&seg6_bpf_srh_states);
> -     struct ipv6_sr_hdr *srh;
>       int srhoff = 0;
> -     int err;
> -
> -     if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
> -             return -EINVAL;
> -     srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
> -
> -     if (!srh_state->valid) {
> -             if (unlikely((srh_state->hdrlen & 7) != 0))
> -                     return -EBADMSG;
> -
> -             srh->hdrlen = (u8)(srh_state->hdrlen >> 3);
> -             if (unlikely(!seg6_validate_srh(srh, (srh->hdrlen + 1) << 3)))
> -                     return -EBADMSG;
>  
> +     if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0) {
> +             srh_state->srh = NULL;
> +     } else {
> +             srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
> +             srh_state->hdrlen = srh_state->srh->hdrlen << 3;
>               srh_state->valid = 1;
>       }
> +}
> +
> +BPF_CALL_4(bpf_lwt_seg6_action, struct sk_buff *, skb,
> +        u32, action, void *, param, u32, param_len)
> +{
> +     struct seg6_bpf_srh_state *srh_state =
> +             this_cpu_ptr(&seg6_bpf_srh_states);
> +     int hdroff = 0;
> +     int err;
>  
>       switch (action) {
>       case SEG6_LOCAL_ACTION_END_X:
> +             if (!seg6_bpf_has_valid_srh(skb))
> +                     return -EBADMSG;
>               if (param_len != sizeof(struct in6_addr))
>                       return -EINVAL;
>               return seg6_lookup_nexthop(skb, (struct in6_addr *)param, 0);
>       case SEG6_LOCAL_ACTION_END_T:
> +             if (!seg6_bpf_has_valid_srh(skb))
> +                     return -EBADMSG;
> +             if (param_len != sizeof(int))
> +                     return -EINVAL;
> +             return seg6_lookup_nexthop(skb, NULL, *(int *)param);
> +     case SEG6_LOCAL_ACTION_END_DT6:
> +             if (!seg6_bpf_has_valid_srh(skb))
> +                     return -EBADMSG;
>               if (param_len != sizeof(int))
>                       return -EINVAL;
> +
> +             // find inner IPv6 header, pull outer IPv6 header
> +             if (ipv6_find_hdr(skb, &hdroff, IPPROTO_IPV6, NULL, NULL) < 0)
> +                     return -EBADMSG;
> +             if (!pskb_pull(skb, hdroff))
> +                     return -EBADMSG;
> +
> +             skb_postpull_rcsum(skb, skb_network_header(skb), hdroff);
> +             skb_reset_network_header(skb);
> +             skb_reset_transport_header(skb);
> +             skb->encapsulation = 0;
> +
> +             bpf_compute_data_pointers(skb);
> +             bpf_update_srh_state(skb);
>               return seg6_lookup_nexthop(skb, NULL, *(int *)param);
>       case SEG6_LOCAL_ACTION_END_B6:
> +             if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
> +                     return -EBADMSG;
>               err = bpf_push_seg6_encap(skb, BPF_LWT_ENCAP_SEG6_INLINE,
>                                         param, param_len);
>               if (!err)
> -                     srh_state->hdrlen =
> -                             ((struct ipv6_sr_hdr *)param)->hdrlen << 3;
> +                     bpf_update_srh_state(skb);
> +
>               return err;
>       case SEG6_LOCAL_ACTION_END_B6_ENCAP:
> +             if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
> +                     return -EBADMSG;
>               err = bpf_push_seg6_encap(skb, BPF_LWT_ENCAP_SEG6,
>                                         param, param_len);
>               if (!err)
> -                     srh_state->hdrlen =
> -                             ((struct ipv6_sr_hdr *)param)->hdrlen << 3;
> +                     bpf_update_srh_state(skb);
> +
>               return err;
>       default:
>               return -EINVAL;
> @@ -4644,15 +4673,14 @@ BPF_CALL_3(bpf_lwt_seg6_adjust_srh, struct sk_buff *, 
> skb, u32, offset,
>  {
>       struct seg6_bpf_srh_state *srh_state =
>               this_cpu_ptr(&seg6_bpf_srh_states);
> +     struct ipv6_sr_hdr *srh = srh_state->srh;
>       void *srh_end, *srh_tlvs, *ptr;
> -     struct ipv6_sr_hdr *srh;
>       struct ipv6hdr *hdr;
>       int srhoff = 0;
>       int ret;
>  
> -     if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
> +     if (unlikely(srh == NULL))
>               return -EINVAL;
> -     srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
>  
>       srh_tlvs = (void *)((unsigned char *)srh + sizeof(*srh) +
>                       ((srh->first_segment + 1) << 4));
> @@ -4682,6 +4710,9 @@ BPF_CALL_3(bpf_lwt_seg6_adjust_srh, struct sk_buff *, 
> skb, u32, offset,
>       hdr = (struct ipv6hdr *)skb->data;
>       hdr->payload_len = htons(skb->len - sizeof(struct ipv6hdr));
>  
> +     if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
> +             return -EINVAL;
> +     srh_state->srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
>       srh_state->hdrlen += len;
>       srh_state->valid = 0;
>       return 0;
> diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
> index e1025b493a18..461f99d5043c 100644
> --- a/net/ipv6/seg6_local.c
> +++ b/net/ipv6/seg6_local.c
> @@ -459,14 +459,35 @@ static int input_action_end_b6_encap(struct sk_buff 
> *skb,
>  
>  DEFINE_PER_CPU(struct seg6_bpf_srh_state, seg6_bpf_srh_states);
>  
> +bool seg6_bpf_has_valid_srh(struct sk_buff *skb)
> +{
> +     struct seg6_bpf_srh_state *srh_state =
> +             this_cpu_ptr(&seg6_bpf_srh_states);
> +     struct ipv6_sr_hdr *srh = srh_state->srh;
> +
> +     if (unlikely(srh == NULL))
> +             return false;
> +
> +     if (unlikely(!srh_state->valid)) {
> +             if ((srh_state->hdrlen & 7) != 0)
> +                     return false;
> +
> +             srh->hdrlen = (u8)(srh_state->hdrlen >> 3);
> +             if (!seg6_validate_srh(srh, (srh->hdrlen + 1) << 3))
> +                     return false;
> +
> +             srh_state->valid = 1;
Nit. s/1/true/

> +     }
> +
> +     return true;
> +}
> +
>  static int input_action_end_bpf(struct sk_buff *skb,
>                               struct seg6_local_lwt *slwt)
>  {
>       struct seg6_bpf_srh_state *srh_state =
>               this_cpu_ptr(&seg6_bpf_srh_states);
> -     struct seg6_bpf_srh_state local_srh_state;
>       struct ipv6_sr_hdr *srh;
> -     int srhoff = 0;
>       int ret;
>  
>       srh = get_and_validate_srh(skb);
> @@ -478,6 +499,7 @@ static int input_action_end_bpf(struct sk_buff *skb,
>        * which is also accessed by the bpf_lwt_seg6_* helpers
>        */
>       preempt_disable();
> +     srh_state->srh = srh;
>       srh_state->hdrlen = srh->hdrlen << 3;
>       srh_state->valid = 1;
>  
> @@ -486,9 +508,6 @@ static int input_action_end_bpf(struct sk_buff *skb,
>       ret = bpf_prog_run_save_cb(slwt->bpf.prog, skb);
>       rcu_read_unlock();
>  
> -     local_srh_state = *srh_state;
> -     preempt_enable();
> -
>       switch (ret) {
>       case BPF_OK:
>       case BPF_REDIRECT:
> @@ -500,24 +519,17 @@ static int input_action_end_bpf(struct sk_buff *skb,
>               goto drop;
>       }
>  
> -     if (unlikely((local_srh_state.hdrlen & 7) != 0))
> -             goto drop;
> -
> -     if (ipv6_find_hdr(skb, &srhoff, IPPROTO_ROUTING, NULL, NULL) < 0)
> -             goto drop;
> -     srh = (struct ipv6_sr_hdr *)(skb->data + srhoff);
> -     srh->hdrlen = (u8)(local_srh_state.hdrlen >> 3);
> -
> -     if (!local_srh_state.valid &&
> -         unlikely(!seg6_validate_srh(srh, (srh->hdrlen + 1) << 3)))
> +     if (srh_state->srh && !seg6_bpf_has_valid_srh(skb))
>               goto drop;
>  
> +     preempt_enable();
>       if (ret != BPF_REDIRECT)
>               seg6_lookup_nexthop(skb, NULL, 0);
>  
>       return dst_input(skb);
>  
>  drop:
> +     preempt_enable();
For this drop case at the beginning of this function:

        srh = get_and_validate_srh(skb);
        if (!srh)
                goto drop;

preempt_disable() was not called yet?

>       kfree_skb(skb);
>       return -EINVAL;
>  }
> -- 
> 2.16.1
> 

Reply via email to