Skip to contentSkip to conversation

ipv4: igmp: guard against silly MTU values

PATCH In review Eric Dumazet 2 participants 2 messages Dec 11, 2017 – Dec 13, 2017

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Eric Dumazet

author
#0 lore raw

PATCH 1/1 ipv4: igmp: guard against silly MTU values

From: Eric Dumazet <[email protected]>

IPv4 stack reacts to changes to small MTU, by disabling itself under RTNL.

But there is a window where threads not using RTNL can see a wrong device mtu. This can lead to surprises, in igmp code where it is assumed the mtu is suitable.

Fix this by reading device mtu once and checking IPv4 minimal MTU.

This patch adds missing IPV4_MIN_MTU define, to not abuse ETH_MIN_MTU anymore.

Signed-off-by Eric Dumazet <[email protected]>

---
 include/net/ip.h     |    1 + net/ipv4/devinet.c   |    2 +- net/ipv4/igmp.c      |   24 +++++++++++++++--------- net/ipv4/ip_tunnel.c |    4 ++--

 4 files changed, 19 insertions(+), 12 deletions(-)
include/net/ip.h +1 -0
index 9896f46cbbf11235395d75a5ec18a14736ee099d..af8addbaa3c188a896b74ff9646b6fdd692d1c8e 100644
@@ -34,6 +34,7 @@
 #include <net/flow_dissector.h>
 
 #define IPV4_MAX_PMTU		65535U		/* RFC 2675, Section 5.1 */
+#define IPV4_MIN_MTU		68			/* RFC 791 */
 
 struct sock;
net/ipv4/devinet.c +1 -1
index a4573bccd6da7b6763016d4f07d3032d44483d99..7a93359fbc7229389fc7bec67889ca1115f47a69 100644
@@ -1428,7 +1428,7 @@ static void inetdev_changename(struct net_device *dev, struct in_device *in_dev)
 
 static bool inetdev_valid_mtu(unsigned int mtu)
 {
-	return mtu >= 68;
+	return mtu >= IPV4_MIN_MTU;
 }
 
 static void inetdev_send_gratuitous_arp(struct net_device *dev,
net/ipv4/igmp.c +15 -9
index d1f8f302dbf3ed5a079f27efa6eeaf802de40243..50448a220a1f26e29715bffc1b86f1f749e5a61d 100644
@@ -404,16 +404,17 @@ static int grec_size(struct ip_mc_list *pmc, int type, int gdel, int sdel)
 }
 
 static struct sk_buff *add_grhead(struct sk_buff *skb, struct ip_mc_list *pmc,
-	int type, struct igmpv3_grec **ppgr)
+	int type, struct igmpv3_grec **ppgr, unsigned int mtu)
 {
 	struct net_device *dev = pmc->interface->dev;
 	struct igmpv3_report *pih;
 	struct igmpv3_grec *pgr;
 
-	if (!skb)
-		skb = igmpv3_newpack(dev, dev->mtu);
-	if (!skb)
-		return NULL;
+	if (!skb) {
+		skb = igmpv3_newpack(dev, mtu);
+		if (!skb)
+			return NULL;
+	}
 	pgr = skb_put(skb, sizeof(struct igmpv3_grec));
 	pgr->grec_type = type;
 	pgr->grec_auxwords = 0;

@@ -436,12 +437,17 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ip_mc_list *pmc,
 	struct igmpv3_grec *pgr = NULL;
 	struct ip_sf_list *psf, *psf_next, *psf_prev, **psf_list;
 	int scount, stotal, first, isquery, truncate;
+	unsigned int mtu;
 
 	if (pmc->multiaddr == IGMP_ALL_HOSTS)
 		return skb;
 	if (ipv4_is_local_multicast(pmc->multiaddr) && !net->ipv4.sysctl_igmp_llm_reports)
 		return skb;
 
+	mtu = READ_ONCE(dev->mtu);
+	if (mtu < IPV4_MIN_MTU)
+		return skb;
+
 	isquery = type == IGMPV3_MODE_IS_INCLUDE ||
 		  type == IGMPV3_MODE_IS_EXCLUDE;
 	truncate = type == IGMPV3_MODE_IS_EXCLUDE ||

@@ -462,7 +468,7 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ip_mc_list *pmc,
 		    AVAILABLE(skb) < grec_size(pmc, type, gdeleted, sdeleted)) {
 			if (skb)
 				igmpv3_sendpack(skb);
-			skb = igmpv3_newpack(dev, dev->mtu);
+			skb = igmpv3_newpack(dev, mtu);
 		}
 	}
 	first = 1;

@@ -498,12 +504,12 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ip_mc_list *pmc,
 				pgr->grec_nsrcs = htons(scount);
 			if (skb)
 				igmpv3_sendpack(skb);
-			skb = igmpv3_newpack(dev, dev->mtu);
+			skb = igmpv3_newpack(dev, mtu);
 			first = 1;
 			scount = 0;
 		}
 		if (first) {
-			skb = add_grhead(skb, pmc, type, &pgr);
+			skb = add_grhead(skb, pmc, type, &pgr, mtu);
 			first = 0;
 		}
 		if (!skb)

@@ -538,7 +544,7 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct ip_mc_list *pmc,
 				igmpv3_sendpack(skb);
 				skb = NULL; /* add_grhead will get a new one */
 			}
-			skb = add_grhead(skb, pmc, type, &pgr);
+			skb = add_grhead(skb, pmc, type, &pgr, mtu);
 		}
 	}
 	if (pgr)
net/ipv4/ip_tunnel.c +2 -2
index fe6fee728ce49d01b55aa478698e1a3bcf9a3bdb..5ddb1cb52bd405ed10cce43195a25607d136efbf 100644
@@ -349,8 +349,8 @@ static int ip_tunnel_bind_dev(struct net_device *dev)
 	dev->needed_headroom = t_hlen + hlen;
 	mtu -= (dev->hard_header_len + t_hlen);
 
-	if (mtu < 68)
-		mtu = 68;
+	if (mtu < IPV4_MIN_MTU)
+		mtu = IPV4_MIN_MTU;
 
 	return mtu;
 }
maintainer
#1 lore raw

From: Eric Dumazet <[email protected]>
Date: Mon, 11 Dec 2017 07:17:39 -0800

Quoted 9 lines

From: Eric Dumazet <[email protected]>

IPv4 stack reacts to changes to small MTU, by disabling itself under RTNL.

But there is a window where threads not using RTNL can see a wrong device mtu. This can lead to surprises, in igmp code where it is assumed the mtu is suitable.

Quoted 6 lines

Fix this by reading device mtu once and checking IPv4 minimal MTU.

This patch adds missing IPV4_MIN_MTU define, to not abuse ETH_MIN_MTU anymore.

Signed-off-by Eric Dumazet <[email protected]>

Applied and queued up for -stable.