lkml.org 
[lkml]   [2019]   [Sep]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.9 06/74] net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list
    Date
    From: Shmulik Ladkani <shmulik@metanetworks.com>

    [ Upstream commit 3dcbdb134f329842a38f0e6797191b885ab00a00 ]

    Historically, support for frag_list packets entering skb_segment() was
    limited to frag_list members terminating on exact same gso_size
    boundaries. This is verified with a BUG_ON since commit 89319d3801d1
    ("net: Add frag_list support to skb_segment"), quote:

    As such we require all frag_list members terminate on exact MSS
    boundaries. This is checked using BUG_ON.
    As there should only be one producer in the kernel of such packets,
    namely GRO, this requirement should not be difficult to maintain.

    However, since commit 6578171a7ff0 ("bpf: add bpf_skb_change_proto helper"),
    the "exact MSS boundaries" assumption no longer holds:
    An eBPF program using bpf_skb_change_proto() DOES modify 'gso_size', but
    leaves the frag_list members as originally merged by GRO with the
    original 'gso_size'. Example of such programs are bpf-based NAT46 or
    NAT64.

    This lead to a kernel BUG_ON for flows involving:
    - GRO generating a frag_list skb
    - bpf program performing bpf_skb_change_proto() or bpf_skb_adjust_room()
    - skb_segment() of the skb

    See example BUG_ON reports in [0].

    In commit 13acc94eff12 ("net: permit skb_segment on head_frag frag_list skb"),
    skb_segment() was modified to support the "gso_size mangling" case of
    a frag_list GRO'ed skb, but *only* for frag_list members having
    head_frag==true (having a page-fragment head).

    Alas, GRO packets having frag_list members with a linear kmalloced head
    (head_frag==false) still hit the BUG_ON.

    This commit adds support to skb_segment() for a 'head_skb' packet having
    a frag_list whose members are *non* head_frag, with gso_size mangled, by
    disabling SG and thus falling-back to copying the data from the given
    'head_skb' into the generated segmented skbs - as suggested by Willem de
    Bruijn [1].

    Since this approach involves the penalty of skb_copy_and_csum_bits()
    when building the segments, care was taken in order to enable this
    solution only when required:
    - untrusted gso_size, by testing SKB_GSO_DODGY is set
    (SKB_GSO_DODGY is set by any gso_size mangling functions in
    net/core/filter.c)
    - the frag_list is non empty, its item is a non head_frag, *and* the
    headlen of the given 'head_skb' does not match the gso_size.

    [0]
    https://lore.kernel.org/netdev/20190826170724.25ff616f@pixies/
    https://lore.kernel.org/netdev/9265b93f-253d-6b8c-f2b8-4b54eff1835c@fb.com/

    [1]
    https://lore.kernel.org/netdev/CA+FuTSfVsgNDi7c=GUU8nMg2hWxF2SjCNLXetHeVPdnxAW5K-w@mail.gmail.com/

    Fixes: 6578171a7ff0 ("bpf: add bpf_skb_change_proto helper")
    Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
    Cc: Daniel Borkmann <daniel@iogearbox.net>
    Cc: Eric Dumazet <eric.dumazet@gmail.com>
    Cc: Alexander Duyck <alexander.duyck@gmail.com>
    Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
    Reviewed-by: Willem de Bruijn <willemb@google.com>
    Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    net/core/skbuff.c | 19 +++++++++++++++++++
    1 file changed, 19 insertions(+)

    --- a/net/core/skbuff.c
    +++ b/net/core/skbuff.c
    @@ -3094,6 +3094,25 @@ struct sk_buff *skb_segment(struct sk_bu
    int pos;
    int dummy;

    + if (list_skb && !list_skb->head_frag && skb_headlen(list_skb) &&
    + (skb_shinfo(head_skb)->gso_type & SKB_GSO_DODGY)) {
    + /* gso_size is untrusted, and we have a frag_list with a linear
    + * non head_frag head.
    + *
    + * (we assume checking the first list_skb member suffices;
    + * i.e if either of the list_skb members have non head_frag
    + * head, then the first one has too).
    + *
    + * If head_skb's headlen does not fit requested gso_size, it
    + * means that the frag_list members do NOT terminate on exact
    + * gso_size boundaries. Hence we cannot perform skb_frag_t page
    + * sharing. Therefore we must fallback to copying the frag_list
    + * skbs; we do so by disabling SG.
    + */
    + if (mss != GSO_BY_FRAGS && mss != skb_headlen(head_skb))
    + features &= ~NETIF_F_SG;
    + }
    +
    __skb_push(head_skb, doffset);
    proto = skb_network_protocol(head_skb, &dummy);
    if (unlikely(!proto))

    \
     
     \ /
      Last update: 2019-09-20 00:20    [W:4.130 / U:0.352 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site