netdev - [PATCH 3/3] openvswitch: drop GSO packets that are too large

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20180116020920.20232-4-dja@axtens.net>
Date:   Tue, 16 Jan 2018 13:09:20 +1100
From:   Daniel Axtens <dja@...ens.net>
To:     netdev@...r.kernel.org
Cc:     Daniel Axtens <dja@...ens.net>, Manish.Chopra@...ium.com,
        dev@...nvswitch.org
Subject: [PATCH 3/3] openvswitch: drop GSO packets that are too large

Open vSwitch attempts to detect if a packet is too large to be
sent to the destination device. However, this test does not
consider GSO packets, and it is possible that a GSO packet, when
resegmented, will be larger than the device can send.

Add detection for packets which are too large. We use
skb_gso_validate_len, reusing the length calculation in the existing
checks - see 738314a084aa
("openvswitch: use hard_header_len instead of hardcoded ETH_HLEN")
This is different from the is_skb_forwardable logic in that it only
allows for the length of a VLAN tag if one is actually present.

Signed-off-by: Daniel Axtens <dja@...ens.net>
---
 net/openvswitch/vport.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index b6c8524032a0..290eeaa82344 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -464,16 +464,16 @@ int ovs_vport_receive(struct vport *vport, struct sk_buff *skb,
 	return 0;
 }
 
-static unsigned int packet_length(const struct sk_buff *skb,
-				  struct net_device *dev)
+static unsigned int packet_length_offset(const struct sk_buff *skb,
+					 const struct net_device *dev)
 {
-	unsigned int length = skb->len - dev->hard_header_len;
+	unsigned int length = dev->hard_header_len;
 
 	if (!skb_vlan_tag_present(skb) &&
 	    eth_type_vlan(skb->protocol))
-		length -= VLAN_HLEN;
+		length += VLAN_HLEN;
 
-	/* Don't subtract for multiple VLAN tags. Most (all?) drivers allow
+	/* Don't adjust for multiple VLAN tags. Most (all?) drivers allow
 	 * (ETH_LEN + VLAN_HLEN) in addition to the mtu value, but almost none
 	 * account for 802.1ad. e.g. is_skb_forwardable().
 	 */
@@ -481,6 +481,21 @@ static unsigned int packet_length(const struct sk_buff *skb,
 	return length;
 }
 
+static inline unsigned int packet_length(const struct sk_buff *skb,
+					 const struct net_device *dev)
+{
+	return skb->len - packet_length_offset(skb, dev);
+}
+
+static inline bool vport_gso_validate_len(const struct sk_buff *skb,
+					  const struct net_device *dev,
+					  unsigned int mtu)
+{
+	unsigned int len = mtu + packet_length_offset(skb, dev);
+	
+	return skb_gso_validate_len(skb, len);
+}
+
 void ovs_vport_send(struct vport *vport, struct sk_buff *skb, u8 mac_proto)
 {
 	int mtu = vport->dev->mtu;
@@ -504,13 +519,21 @@ void ovs_vport_send(struct vport *vport, struct sk_buff *skb, u8 mac_proto)
 		goto drop;
 	}
 
-	if (unlikely(packet_length(skb, vport->dev) > mtu &&
-		     !skb_is_gso(skb))) {
+	if (!skb_is_gso(skb) &&
+	    unlikely(packet_length(skb, vport->dev) > mtu)) {
 		net_warn_ratelimited("%s: dropped over-mtu packet: %d > %d\n",
 				     vport->dev->name,
 				     packet_length(skb, vport->dev), mtu);
 		vport->dev->stats.tx_errors++;
 		goto drop;
+	} else if (skb_is_gso(skb) &&
+		   unlikely(!vport_gso_validate_len(skb, vport->dev, mtu))) {
+		net_warn_ratelimited("%s: dropped over-mtu GSO packet: "
+				     "gso_size = %d, mtu = %d\n",
+				     vport->dev->name,
+				     skb_shinfo(skb)->gso_size, mtu);
+		vport->dev->stats.tx_errors++;
+		goto drop;
 	}
 
 	skb->dev = vport->dev;
-- 
2.14.1