Commit cb9f1b78 authored by Willem de Bruijn's avatar Willem de Bruijn Committed by David S. Miller
Browse files

ip: validate header length on virtual device xmit 



KMSAN detected read beyond end of buffer in vti and sit devices when
passing truncated packets with PF_PACKET. The issue affects additional
ip tunnel devices.

Extend commit 76c0ddd8 ("ip6_tunnel: be careful when accessing the
inner header") and commit ccfec9e5 ("ip_tunnel: be careful when
accessing the inner header").

Move the check to a separate helper and call at the start of each
ndo_start_xmit function in net/ipv4 and net/ipv6.

Minor changes:
- convert dev_kfree_skb to kfree_skb on error path,
  as dev_kfree_skb calls consume_skb which is not for error paths.
- use pskb_network_may_pull even though that is pedantic here,
  as the same as pskb_may_pull for devices without llheaders.
- do not cache ipv6 hdrs if used only once
  (unsafe across pskb_may_pull, was more relevant to earlier patch)

Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
Signed-off-by: default avatarWillem de Bruijn <willemb@google.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 8c76e77f

include/net/ip_tunnels.h

+20 −0
Original line number Diff line number Diff line
@@ -308,6 +308,26 @@ int ip_tunnel_encap_del_ops(const struct ip_tunnel_encap_ops *op, 
int ip_tunnel_encap_setup(struct ip_tunnel *t,

			  struct ip_tunnel_encap *ipencap);



static inline bool pskb_inet_may_pull(struct sk_buff *skb)

{

	int nhlen;



	switch (skb->protocol) {

#if IS_ENABLED(CONFIG_IPV6)

	case htons(ETH_P_IPV6):

		nhlen = sizeof(struct ipv6hdr);

		break;

#endif

	case htons(ETH_P_IP):

		nhlen = sizeof(struct iphdr);

		break;

	default:

		nhlen = 0;

	}



	return pskb_network_may_pull(skb, nhlen);

}



static inline int ip_encap_hlen(struct ip_tunnel_encap *e)

{

	const struct ip_tunnel_encap_ops *ops;

net/ipv4/ip_gre.c

+9 −0
Original line number Diff line number Diff line
@@ -676,6 +676,9 @@ static netdev_tx_t ipgre_xmit(struct sk_buff *skb, 
	struct ip_tunnel *tunnel = netdev_priv(dev);

	const struct iphdr *tnl_params;



	if (!pskb_inet_may_pull(skb))

		goto free_skb;



	if (tunnel->collect_md) {

		gre_fb_xmit(skb, dev, skb->protocol);

		return NETDEV_TX_OK;
@@ -719,6 +722,9 @@ static netdev_tx_t erspan_xmit(struct sk_buff *skb, 
	struct ip_tunnel *tunnel = netdev_priv(dev);

	bool truncate = false;



	if (!pskb_inet_may_pull(skb))

		goto free_skb;



	if (tunnel->collect_md) {

		erspan_fb_xmit(skb, dev, skb->protocol);

		return NETDEV_TX_OK;
@@ -762,6 +768,9 @@ static netdev_tx_t gre_tap_xmit(struct sk_buff *skb, 
{

	struct ip_tunnel *tunnel = netdev_priv(dev);



	if (!pskb_inet_may_pull(skb))

		goto free_skb;



	if (tunnel->collect_md) {

		gre_fb_xmit(skb, dev, htons(ETH_P_TEB));

		return NETDEV_TX_OK;

net/ipv4/ip_tunnel.c

+0 −9
Original line number Diff line number Diff line
@@ -627,7 +627,6 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev, 
		    const struct iphdr *tnl_params, u8 protocol)

{

	struct ip_tunnel *tunnel = netdev_priv(dev);

	unsigned int inner_nhdr_len = 0;

	const struct iphdr *inner_iph;

	struct flowi4 fl4;

	u8     tos, ttl;
@@ -637,14 +636,6 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev, 
	__be32 dst;

	bool connected;



	/* ensure we can access the inner net header, for several users below */

	if (skb->protocol == htons(ETH_P_IP))

		inner_nhdr_len = sizeof(struct iphdr);

	else if (skb->protocol == htons(ETH_P_IPV6))

		inner_nhdr_len = sizeof(struct ipv6hdr);

	if (unlikely(!pskb_may_pull(skb, inner_nhdr_len)))

		goto tx_error;



	inner_iph = (const struct iphdr *)skb_inner_network_header(skb);

	connected = (tunnel->parms.iph.daddr != 0);

net/ipv4/ip_vti.c

+9 −3
Original line number Diff line number Diff line
@@ -241,6 +241,9 @@ static netdev_tx_t vti_tunnel_xmit(struct sk_buff *skb, struct net_device *dev) 
	struct ip_tunnel *tunnel = netdev_priv(dev);

	struct flowi fl;



	if (!pskb_inet_may_pull(skb))

		goto tx_err;



	memset(&fl, 0, sizeof(fl));



	switch (skb->protocol) {
@@ -253,15 +256,18 @@ static netdev_tx_t vti_tunnel_xmit(struct sk_buff *skb, struct net_device *dev) 
		memset(IP6CB(skb), 0, sizeof(*IP6CB(skb)));

		break;

	default:

		dev->stats.tx_errors++;

		dev_kfree_skb(skb);

		return NETDEV_TX_OK;

		goto tx_err;

	}



	/* override mark with tunnel output key */

	fl.flowi_mark = be32_to_cpu(tunnel->parms.o_key);



	return vti_xmit(skb, dev, &fl);



tx_err:

	dev->stats.tx_errors++;

	kfree_skb(skb);

	return NETDEV_TX_OK;

}



static int vti4_err(struct sk_buff *skb, u32 info)

net/ipv6/ip6_gre.c

+7 −3
Original line number Diff line number Diff line
@@ -881,6 +881,9 @@ static netdev_tx_t ip6gre_tunnel_xmit(struct sk_buff *skb, 
	struct net_device_stats *stats = &t->dev->stats;

	int ret;



	if (!pskb_inet_may_pull(skb))

		goto tx_err;



	if (!ip6_tnl_xmit_ctl(t, &t->parms.laddr, &t->parms.raddr))

		goto tx_err;
@@ -923,6 +926,9 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb, 
	int nhoff;

	int thoff;



	if (!pskb_inet_may_pull(skb))

		goto tx_err;



	if (!ip6_tnl_xmit_ctl(t, &t->parms.laddr, &t->parms.raddr))

		goto tx_err;
@@ -995,8 +1001,6 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb, 
			goto tx_err;

		}

	} else {

		struct ipv6hdr *ipv6h = ipv6_hdr(skb);



		switch (skb->protocol) {

		case htons(ETH_P_IP):

			memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
@@ -1004,7 +1008,7 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb, 
						 &dsfield, &encap_limit);

			break;

		case htons(ETH_P_IPV6):

			if (ipv6_addr_equal(&t->parms.raddr, &ipv6h->saddr))

			if (ipv6_addr_equal(&t->parms.raddr, &ipv6_hdr(skb)->saddr))

				goto tx_err;

			if (prepare_ip6gre_xmit_ipv6(skb, dev, &fl6,

						     &dsfield, &encap_limit))

CRA Git | Maintained and supported by SUSTech CRA and CCSE