Commit 8a0ed250 authored by Willem de Bruijn's avatar Willem de Bruijn Committed by David S. Miller

ip_gre: validate csum_start only on pull

The GRE tunnel device can pull existing outer headers in ipge_xmit.
This is a rare path, apparently unique to this device. The below
commit ensured that pulling does not move skb->data beyond csum_start.

But it has a false positive if ip_summed is not CHECKSUM_PARTIAL and
thus csum_start is irrelevant.

Refine to exclude this. At the same time simplify and strengthen the
test.

Simplify, by moving the check next to the offending pull, making it
more self documenting and removing an unnecessary branch from other
code paths.

Strengthen, by also ensuring that the transport header is correct and
therefore the inner headers will be after skb_reset_inner_headers.
The transport header is set to csum_start in skb_partial_csum_set.

Link: https://lore.kernel.org/netdev/YS+h%2FtqCJJiQei+W@shredder/
Fixes: 1d011c48 ("ip_gre: add validation for csum_start")
Reported-by: default avatarIdo Schimmel <idosch@idosch.org>
Suggested-by: default avatarAlexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: default avatarWillem de Bruijn <willemb@google.com>
Reviewed-by: default avatarAlexander Duyck <alexanderduyck@fb.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 0a4fd8df
...@@ -473,8 +473,6 @@ static void __gre_xmit(struct sk_buff *skb, struct net_device *dev, ...@@ -473,8 +473,6 @@ static void __gre_xmit(struct sk_buff *skb, struct net_device *dev,
static int gre_handle_offloads(struct sk_buff *skb, bool csum) static int gre_handle_offloads(struct sk_buff *skb, bool csum)
{ {
if (csum && skb_checksum_start(skb) < skb->data)
return -EINVAL;
return iptunnel_handle_offloads(skb, csum ? SKB_GSO_GRE_CSUM : SKB_GSO_GRE); return iptunnel_handle_offloads(skb, csum ? SKB_GSO_GRE_CSUM : SKB_GSO_GRE);
} }
...@@ -632,15 +630,20 @@ static netdev_tx_t ipgre_xmit(struct sk_buff *skb, ...@@ -632,15 +630,20 @@ static netdev_tx_t ipgre_xmit(struct sk_buff *skb,
} }
if (dev->header_ops) { if (dev->header_ops) {
const int pull_len = tunnel->hlen + sizeof(struct iphdr);
if (skb_cow_head(skb, 0)) if (skb_cow_head(skb, 0))
goto free_skb; goto free_skb;
tnl_params = (const struct iphdr *)skb->data; 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 /* Pull skb since ip_tunnel_xmit() needs skb->data pointing
* to gre header. * to gre header.
*/ */
skb_pull(skb, tunnel->hlen + sizeof(struct iphdr)); skb_pull(skb, pull_len);
skb_reset_mac_header(skb); skb_reset_mac_header(skb);
} else { } else {
if (skb_cow_head(skb, dev->needed_headroom)) if (skb_cow_head(skb, dev->needed_headroom))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment