| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20180123224239.11772-1-code@rkapl.cz>
Date: Tue, 23 Jan 2018 23:42:39 +0100
From: Roman Kapl <code@...pl.cz>
To: netdev@...r.kernel.org
Cc: "David S . Miller" <davem@...emloft.net>,
Jiri Benc <jbenc@...hat.com>, Xin Long <lucien.xin@...il.com>,
Roman Kapl <code@...pl.cz>
Subject: [PATCH] net: make sure skb_dst is valid before using
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@...pl.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
Powered by blists - more mailing lists