lkml.org 
[lkml]   [2022]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.10 110/172] ip_gre: test csum_start instead of transport header
    Date
    From: Willem de Bruijn <willemb@google.com>

    [ Upstream commit 8d21e9963bec1aad2280cdd034c8993033ef2948 ]

    GRE with TUNNEL_CSUM will apply local checksum offload on
    CHECKSUM_PARTIAL packets.

    ipgre_xmit must validate csum_start after an optional skb_pull,
    else lco_csum may trigger an overflow. The original check was

    if (csum && skb_checksum_start(skb) < skb->data)
    return -EINVAL;

    This had false positives when skb_checksum_start is undefined:
    when ip_summed is not CHECKSUM_PARTIAL. A discussed refinement
    was straightforward

    if (csum && skb->ip_summed == CHECKSUM_PARTIAL &&
    skb_checksum_start(skb) < skb->data)
    return -EINVAL;

    But was eventually revised more thoroughly:
    - restrict the check to the only branch where needed, in an
    uncommon GRE path that uses header_ops and calls skb_pull.
    - test skb_transport_header, which is set along with csum_start
    in skb_partial_csum_set in the normal header_ops datapath.

    Turns out skbs can arrive in this branch without the transport
    header set, e.g., through BPF redirection.

    Revise the check back to check csum_start directly, and only if
    CHECKSUM_PARTIAL. Do leave the check in the updated location.
    Check field regardless of whether TUNNEL_CSUM is configured.

    Link: https://lore.kernel.org/netdev/YS+h%2FtqCJJiQei+W@shredder/
    Link: https://lore.kernel.org/all/20210902193447.94039-2-willemdebruijn.kernel@gmail.com/T/#u
    Fixes: 8a0ed250f911 ("ip_gre: validate csum_start only on pull")
    Reported-by: syzbot <syzkaller@googlegroups.com>
    Signed-off-by: Willem de Bruijn <willemb@google.com>
    Reviewed-by: Eric Dumazet <edumazet@google.com>
    Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
    Link: https://lore.kernel.org/r/20220606132107.3582565-1-willemdebruijn.kernel@gmail.com
    Signed-off-by: Jakub Kicinski <kuba@kernel.org>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    net/ipv4/ip_gre.c | 11 +++++------
    1 file changed, 5 insertions(+), 6 deletions(-)

    diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
    index 2a80038575d2..a7e32be8714f 100644
    --- a/net/ipv4/ip_gre.c
    +++ b/net/ipv4/ip_gre.c
    @@ -624,21 +624,20 @@ static netdev_tx_t ipgre_xmit(struct sk_buff *skb,
    }

    if (dev->header_ops) {
    - const int pull_len = tunnel->hlen + sizeof(struct iphdr);
    -
    if (skb_cow_head(skb, 0))
    goto free_skb;

    tnl_params = (const struct iphdr *)skb->data;

    - if (pull_len > skb_transport_offset(skb))
    - goto free_skb;
    -
    /* Pull skb since ip_tunnel_xmit() needs skb->data pointing
    * to gre header.
    */
    - skb_pull(skb, pull_len);
    + skb_pull(skb, tunnel->hlen + sizeof(struct iphdr));
    skb_reset_mac_header(skb);
    +
    + if (skb->ip_summed == CHECKSUM_PARTIAL &&
    + skb_checksum_start(skb) < skb->data)
    + goto free_skb;
    } else {
    if (skb_cow_head(skb, dev->needed_headroom))
    goto free_skb;
    --
    2.35.1


    \
     
     \ /
      Last update: 2022-06-13 14:46    [W:4.085 / U:0.752 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site