Skip to content
  • Daniel Borkmann's avatar
    bpf: fix write helpers with regards to non-linear parts · 0ed661d5
    Daniel Borkmann authored
    Fix the bpf_try_make_writable() helper and all call sites we have in BPF,
    it's currently defect with regards to skbs when the write_len spans into
    non-linear parts, no matter if cloned or not.
    
    There are multiple issues at once. First, using skb_store_bits() is not
    correct since even if we have a cloned skb, page frags can still be shared.
    To really make them private, we need to pull them in via __pskb_pull_tail()
    first, which also gets us a private head via pskb_expand_head() implicitly.
    
    This is for helpers like bpf_skb_store_bytes(), bpf_l3_csum_replace(),
    bpf_l4_csum_replace(). Really, the only thing reasonable and working here
    is to call skb_ensure_writable() before any write operation. Meaning, via
    pskb_may_pull() it makes sure that parts we want to access are pulled in and
    if not does so plus unclones the skb implicitly. If our write_len still fits
    the headlen and we're cloned and our header of the clone is not writable,
    then we need to make a private copy via pskb_expand_head(). skb_store_bits()
    is a bit misleading and only safe to store into non-linear data in different
    contexts such as 357b40a1 ("[IPV6]: IPV6_CHECKSUM socket option can
    corrupt kernel memory").
    
    For above BPF helper functions, it means after fixed bpf_try_make_writable(),
    we've pulled in enough, so that we operate always based on skb->data. Thus,
    the call to skb_header_pointer() and skb_store_bits() becomes superfluous.
    In bpf_skb_store_bytes(), the len check is unnecessary too since it can
    only pass in maximum of BPF stack size, so adding offset is guaranteed to
    never overflow. Also bpf_l3/4_csum_replace() helpers must test for proper
    offset alignment since they use __sum16 pointer for writing resulting csum.
    
    The remaining helpers that change skb data not discussed here yet are
    bpf_skb_vlan_push(), bpf_skb_vlan_pop() and bpf_skb_change_proto(). The
    vlan helpers internally call either skb_ensure_writable() (pop case) and
    skb_cow_head() (push case, for head expansion), respectively. Similarly,
    bpf_skb_proto_xlat() takes care to not mangle page frags.
    
    Fixes: 608cd71a ("tc: bpf: generalize pedit action")
    Fixes: 91bc4822 ("tc: bpf: add checksum helpers")
    Fixes: 3697649f
    
     ("bpf: try harder on clones when writing into skb")
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Acked-by: default avatarAlexei Starovoitov <ast@kernel.org>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    0ed661d5