From: Roman Kapl <code@rkapl.cz>
To: netdev@vger.kernel.org
Cc: "David S . Miller" <davem@davemloft.net>,
Jiri Benc <jbenc@redhat.com>, Xin Long <lucien.xin@gmail.com>,
Roman Kapl <code@rkapl.cz>
Subject: [PATCH] net: make sure skb_dst is valid before using
Date: Tue, 23 Jan 2018 23:42:39 +0100 [thread overview]
Message-ID: <20180123224239.11772-1-code@rkapl.cz> (raw)
Tunnel devices often use skb_dst(skb)->ops, but ops are not implemented
for metadata tunnel destinations. Use skb_valid_dst to check if skb_dst
is a real (non-metadata) destination.
Such packets can easily be crafted using tc tunnel_key actions or BPF
and will crash the tunnels, as observed at least with sit, geneve and
vxlan. Some of these tunnels are also intended to be used with metadata
destinations, so this represents a loss of functionality.
There might be more places with similar patterns, but sometimes it is
hard to tell if metadata dst packets can reach them, so this patch is
not exhaustive.
Fixes: 52a589d51f10 ("geneve: update skb dst pmtu on tx path")
Fixes: a93bf0ff4490 ("vxlan: update skb dst pmtu on tx path")
Signed-off-by: Roman Kapl <code@rkapl.cz>
---
drivers/infiniband/ulp/ipoib/ipoib_cm.c | 3 ++-
drivers/net/geneve.c | 5 +++--
drivers/net/vxlan.c | 5 +++--
drivers/s390/net/qeth_l3_main.c | 7 +++++--
include/net/ip6_tunnel.h | 1 +
net/ipv4/ip_tunnel.c | 2 +-
net/ipv4/ip_vti.c | 3 ++-
net/ipv6/sit.c | 7 ++++---
8 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
index 2c13123bfd69..65eacafc898f 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
@@ -40,6 +40,7 @@
#include <linux/moduleparam.h>
#include <linux/sched/signal.h>
#include <linux/sched/mm.h>
+#include <net/dst_metadata.h>
#include "ipoib.h"
@@ -1456,7 +1457,7 @@ void ipoib_cm_skb_too_long(struct net_device *dev, struct sk_buff *skb,
struct ipoib_dev_priv *priv = ipoib_priv(dev);
int e = skb_queue_empty(&priv->cm.skb_queue);
- if (skb_dst(skb))
+ if (skb_valid_dst(skb))
skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu);
skb_queue_tail(&priv->cm.skb_queue, skb);
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 195e0d0add8d..2e5d2bc8d851 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -19,6 +19,7 @@
#include <net/rtnetlink.h>
#include <net/geneve.h>
#include <net/protocol.h>
+#include <net/dst_metadata.h>
#define GENEVE_NETDEV_VER "0.6"
@@ -825,7 +826,7 @@ static int geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
if (IS_ERR(rt))
return PTR_ERR(rt);
- if (skb_dst(skb)) {
+ if (skb_valid_dst(skb)) {
int mtu = dst_mtu(&rt->dst) - sizeof(struct iphdr) -
GENEVE_BASE_HLEN - info->options_len - 14;
@@ -871,7 +872,7 @@ static int geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
if (IS_ERR(dst))
return PTR_ERR(dst);
- if (skb_dst(skb)) {
+ if (skb_valid_dst(skb)) {
int mtu = dst_mtu(dst) - sizeof(struct ipv6hdr) -
GENEVE_BASE_HLEN - info->options_len - 14;
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 82090ae7ced1..b0e96746bc2e 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -28,6 +28,7 @@
#include <net/netns/generic.h>
#include <net/tun_proto.h>
#include <net/vxlan.h>
+#include <net/dst_metadata.h>
#if IS_ENABLED(CONFIG_IPV6)
#include <net/ip6_tunnel.h>
@@ -2155,7 +2156,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
}
ndst = &rt->dst;
- if (skb_dst(skb)) {
+ if (skb_valid_dst(skb)) {
int mtu = dst_mtu(ndst) - VXLAN_HEADROOM;
skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL,
@@ -2197,7 +2198,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
goto out_unlock;
}
- if (skb_dst(skb)) {
+ if (skb_valid_dst(skb)) {
int mtu = dst_mtu(ndst) - VXLAN6_HEADROOM;
skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL,
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index b0c888e86cd4..694efab9cccd 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -35,6 +35,7 @@
#include <net/ip6_fib.h>
#include <net/ip6_checksum.h>
#include <net/iucv/af_iucv.h>
+#include <net/dst_metadata.h>
#include <linux/hashtable.h>
#include "qeth_l3.h"
@@ -2259,9 +2260,11 @@ static int qeth_l3_get_cast_type(struct sk_buff *skb)
struct dst_entry *dst;
rcu_read_lock();
- dst = skb_dst(skb);
- if (dst)
+ if (skb_valid_dst(skb)) {
+ dst = skb_dst(skb);
n = dst_neigh_lookup_skb(dst, skb);
+ }
+
if (n) {
int cast_type = n->type;
diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h
index 236e40ba06bf..1a704dfe5e1a 100644
--- a/include/net/ip6_tunnel.h
+++ b/include/net/ip6_tunnel.h
@@ -148,6 +148,7 @@ struct net *ip6_tnl_get_link_net(const struct net_device *dev);
int ip6_tnl_get_iflink(const struct net_device *dev);
int ip6_tnl_change_mtu(struct net_device *dev, int new_mtu);
+/* Must be called with valid skb_dst */
static inline void ip6tunnel_xmit(struct sock *sk, struct sk_buff *skb,
struct net_device *dev)
{
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 5ddb1cb52bd4..9256e8b9c538 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -520,7 +520,7 @@ static int tnl_update_pmtu(struct net_device *dev, struct sk_buff *skb,
else
mtu = skb_dst(skb) ? dst_mtu(skb_dst(skb)) : dev->mtu;
- if (skb_dst(skb))
+ if (skb_valid_dst(skb))
skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu);
if (skb->protocol == htons(ETH_P_IP)) {
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 949f432a5f04..fdb20a95b819 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -43,6 +43,7 @@
#include <net/xfrm.h>
#include <net/net_namespace.h>
#include <net/netns/generic.h>
+#include <net/dst_metadata.h>
static struct rtnl_link_ops vti_link_ops __read_mostly;
@@ -172,7 +173,7 @@ static netdev_tx_t vti_xmit(struct sk_buff *skb, struct net_device *dev,
int err;
int mtu;
- if (!dst) {
+ if (!skb_valid_dst(skb)) {
dev->stats.tx_carrier_errors++;
goto tx_error_icmp;
}
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index d7dc23c1b2ca..50a5be98462f 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -55,6 +55,7 @@
#include <net/dsfield.h>
#include <net/net_namespace.h>
#include <net/netns/generic.h>
+#include <net/dst_metadata.h>
/*
This version of net/ipv6/sit.c is cloned of net/ipv4/ip_gre.c
@@ -837,7 +838,7 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
struct neighbour *neigh = NULL;
bool do_tx_error = false;
- if (skb_dst(skb))
+ if (skb_valid_dst(skb))
neigh = dst_neigh_lookup(skb_dst(skb), &iph6->daddr);
if (!neigh) {
@@ -866,7 +867,7 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
struct neighbour *neigh = NULL;
bool do_tx_error = false;
- if (skb_dst(skb))
+ if (skb_valid_dst(skb))
neigh = dst_neigh_lookup(skb_dst(skb), &iph6->daddr);
if (!neigh) {
@@ -934,7 +935,7 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
df = 0;
}
- if (tunnel->parms.iph.daddr && skb_dst(skb))
+ if (tunnel->parms.iph.daddr && skb_valid_dst(skb))
skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu);
if (skb->len > mtu && !skb_is_gso(skb)) {
--
2.15.1
next reply other threads:[~2018-01-23 22:42 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-23 22:42 Roman Kapl [this message]
2018-01-24 9:16 ` [PATCH] net: make sure skb_dst is valid before using Xin Long
2018-01-24 9:49 ` Roman Kapl
2018-01-24 15:37 ` Nicolas Dichtel
2018-01-25 16:25 ` David Miller
2018-01-25 18:03 ` [PATCH net] net: don't call update_pmtu unconditionally Nicolas Dichtel
2018-01-25 18:08 ` David Ahern
2018-01-25 21:28 ` David Miller
2018-01-25 23:10 ` Nicolas Dichtel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180123224239.11772-1-code@rkapl.cz \
--to=code@rkapl.cz \
--cc=davem@davemloft.net \
--cc=jbenc@redhat.com \
--cc=lucien.xin@gmail.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).