diff mbox series

batman-adv: simplify ip_mc_check_igmp() and ipv6_mc_check_mld() calls

Message ID 20190125080132.10524-1-sven@narfation.org (mailing list archive)
State Accepted, archived
Commit f519588d53e2a7d8b170df24e172029f8d085dc8
Delegated to: Simon Wunderlich
Headers
Series batman-adv: simplify ip_mc_check_igmp() and ipv6_mc_check_mld() calls |

Commit Message

Sven Eckelmann Jan. 25, 2019, 8:01 a.m. UTC 
  From: Linus Lüssing <linus.luessing@c0d3.blue>

This patch refactors ip_mc_check_igmp(), ipv6_mc_check_mld() and
their callers (more precisely, the Linux bridge) to not rely on
the skb_trimmed parameter anymore.

An skb with its tail trimmed to the IP packet length was initially
introduced for the following three reasons:

1) To be able to verify the ICMPv6 checksum.
2) To be able to distinguish the version of an IGMP or MLD query.
   They are distinguishable only by their size.
3) To avoid parsing data for an IGMPv3 or MLDv2 report that is
   beyond the IP packet but still within the skb.

The first case still uses a cloned and potentially trimmed skb to
verfiy. However, there is no need to propagate it to the caller.
For the second and third case explicit IP packet length checks were
added.

This hopefully makes ip_mc_check_igmp() and ipv6_mc_check_mld() easier
to read and verfiy, as well as easier to use.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: David S. Miller <davem@davemloft.net>
[sven@narfation.org: Added compat code]
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 compat-include/linux/igmp.h           | 12 +++++++++++-
 compat-include/net/addrconf.h         | 12 +++++++++++-
 compat-sources/net/ipv4/igmp.c        | 22 ++++------------------
 compat-sources/net/ipv6/mcast_snoop.c | 23 ++++-------------------
 net/batman-adv/multicast.c            |  4 ++--
 5 files changed, 32 insertions(+), 41 deletions(-)
diff mbox series

Patch

diff --git a/compat-include/linux/igmp.h b/compat-include/linux/igmp.h
index 59f55522..3aa64197 100644
--- a/compat-include/linux/igmp.h
+++ b/compat-include/linux/igmp.h
@@ -27,7 +27,17 @@ 
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(4, 2, 0)
 
-int ip_mc_check_igmp(struct sk_buff *skb, struct sk_buff **skb_trimmed);
+int ip_mc_check_igmp(struct sk_buff *skb);
+
+#elif LINUX_VERSION_CODE < KERNEL_VERSION(5, 1, 0)
+
+static inline int batadv_ip_mc_check_igmp(struct sk_buff *skb)
+{
+	return ip_mc_check_igmp(skb, NULL);
+}
+
+#define ip_mc_check_igmp(skb) \
+	batadv_ip_mc_check_igmp(skb)
 
 #endif /* < KERNEL_VERSION(4, 2, 0) */
 
diff --git a/compat-include/net/addrconf.h b/compat-include/net/addrconf.h
index ca9d6935..a37a8e9f 100644
--- a/compat-include/net/addrconf.h
+++ b/compat-include/net/addrconf.h
@@ -27,7 +27,17 @@ 
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(4, 2, 0)
 
-int ipv6_mc_check_mld(struct sk_buff *skb, struct sk_buff **skb_trimmed);
+int ipv6_mc_check_mld(struct sk_buff *skb);
+
+#elif LINUX_VERSION_CODE < KERNEL_VERSION(5, 1, 0)
+
+static inline int batadv_ipv6_mc_check_mld(struct sk_buff *skb)
+{
+	return ipv6_mc_check_mld(skb, NULL);
+}
+
+#define ipv6_mc_check_mld(skb) \
+	batadv_ipv6_mc_check_mld(skb)
 
 #endif /* < KERNEL_VERSION(4, 2, 0) */
 
diff --git a/compat-sources/net/ipv4/igmp.c b/compat-sources/net/ipv4/igmp.c
index 5eb149b7..ecb987d1 100644
--- a/compat-sources/net/ipv4/igmp.c
+++ b/compat-sources/net/ipv4/igmp.c
@@ -168,7 +168,7 @@  static inline __sum16 ip_mc_validate_checksum(struct sk_buff *skb)
 	return skb_checksum_simple_validate(skb);
 }
 
-static int __ip_mc_check_igmp(struct sk_buff *skb, struct sk_buff **skb_trimmed)
+static int __ip_mc_check_igmp(struct sk_buff *skb)
 
 {
 	struct sk_buff *skb_chk;
@@ -195,10 +195,7 @@  static int __ip_mc_check_igmp(struct sk_buff *skb, struct sk_buff **skb_trimmed)
 		return ret;
 	}
 
-	if (skb_trimmed)
-		*skb_trimmed = skb_chk;
-	else
-		kfree_skb(skb_chk);
+	kfree_skb(skb_chk);
 
 	return 0;
 }
@@ -206,7 +203,6 @@  static int __ip_mc_check_igmp(struct sk_buff *skb, struct sk_buff **skb_trimmed)
 /**
  * ip_mc_check_igmp - checks whether this is a sane IGMP packet
  * @skb: the skb to validate
- * @skb_trimmed: to store an skb pointer trimmed to IPv4 packet tail (optional)
  *
  * Checks whether an IPv4 packet is a valid IGMP packet. If so sets
  * skb network and transport headers accordingly and returns zero.
@@ -215,18 +211,8 @@  static int __ip_mc_check_igmp(struct sk_buff *skb, struct sk_buff **skb_trimmed)
  *  standard
  * -ENOMSG: IP header validation succeeded but it is not an IGMP packet.
  * -ENOMEM: A memory allocation failure happened.
- *
- * Optionally, an skb pointer might be provided via skb_trimmed (or set it
- * to NULL): After parsing an IGMP packet successfully it will point to
- * an skb which has its tail aligned to the IP packet end. This might
- * either be the originally provided skb or a trimmed, cloned version if
- * the skb frame had data beyond the IP packet. A cloned skb allows us
- * to leave the original skb and its full frame unchanged (which might be
- * desirable for layer 2 frame jugglers).
- *
- * The caller needs to release a reference count from any returned skb_trimmed.
  */
-int ip_mc_check_igmp(struct sk_buff *skb, struct sk_buff **skb_trimmed)
+int ip_mc_check_igmp(struct sk_buff *skb)
 {
 	int ret = ip_mc_check_iphdr(skb);
 
@@ -236,7 +222,7 @@  int ip_mc_check_igmp(struct sk_buff *skb, struct sk_buff **skb_trimmed)
 	if (ip_hdr(skb)->protocol != IPPROTO_IGMP)
 		return -ENOMSG;
 
-	return __ip_mc_check_igmp(skb, skb_trimmed);
+	return __ip_mc_check_igmp(skb);
 }
 
 #endif /* < KERNEL_VERSION(4, 2, 0) */
diff --git a/compat-sources/net/ipv6/mcast_snoop.c b/compat-sources/net/ipv6/mcast_snoop.c
index 8c380034..01b19969 100644
--- a/compat-sources/net/ipv6/mcast_snoop.c
+++ b/compat-sources/net/ipv6/mcast_snoop.c
@@ -139,8 +139,7 @@  static inline __sum16 ipv6_mc_validate_checksum(struct sk_buff *skb)
 	return skb_checksum_validate(skb, IPPROTO_ICMPV6, ip6_compute_pseudo);
 }
 
-static int __ipv6_mc_check_mld(struct sk_buff *skb,
-			       struct sk_buff **skb_trimmed)
+static int __ipv6_mc_check_mld(struct sk_buff *skb)
 
 {
 	struct sk_buff *skb_chk = NULL;
@@ -168,10 +167,7 @@  static int __ipv6_mc_check_mld(struct sk_buff *skb,
 		return ret;
 	}
 
-	if (skb_trimmed)
-		*skb_trimmed = skb_chk;
-	else
-		kfree_skb(skb_chk);
+	kfree_skb(skb_chk);
 
 	return 0;
 }
@@ -179,7 +175,6 @@  static int __ipv6_mc_check_mld(struct sk_buff *skb,
 /**
  * ipv6_mc_check_mld - checks whether this is a sane MLD packet
  * @skb: the skb to validate
- * @skb_trimmed: to store an skb pointer trimmed to IPv6 packet tail (optional)
  *
  * Checks whether an IPv6 packet is a valid MLD packet. If so sets
  * skb network and transport headers accordingly and returns zero.
@@ -188,18 +183,8 @@  static int __ipv6_mc_check_mld(struct sk_buff *skb,
  *  standard
  * -ENOMSG: IP header validation succeeded but it is not an MLD packet.
  * -ENOMEM: A memory allocation failure happened.
- *
- * Optionally, an skb pointer might be provided via skb_trimmed (or set it
- * to NULL): After parsing an MLD packet successfully it will point to
- * an skb which has its tail aligned to the IP packet end. This might
- * either be the originally provided skb or a trimmed, cloned version if
- * the skb frame had data beyond the IP packet. A cloned skb allows us
- * to leave the original skb and its full frame unchanged (which might be
- * desirable for layer 2 frame jugglers).
- *
- * The caller needs to release a reference count from any returned skb_trimmed.
  */
-int ipv6_mc_check_mld(struct sk_buff *skb, struct sk_buff **skb_trimmed)
+int ipv6_mc_check_mld(struct sk_buff *skb)
 {
 	int ret;
 
@@ -211,7 +196,7 @@  int ipv6_mc_check_mld(struct sk_buff *skb, struct sk_buff **skb_trimmed)
 	if (ret < 0)
 		return ret;
 
-	return __ipv6_mc_check_mld(skb, skb_trimmed);
+	return __ipv6_mc_check_mld(skb);
 }
 
 #endif /* < KERNEL_VERSION(4, 2, 0) */
diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
index 5de6a375..f91b1b62 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -674,7 +674,7 @@  static void batadv_mcast_mla_update(struct work_struct *work)
  */
 static bool batadv_mcast_is_report_ipv4(struct sk_buff *skb)
 {
-	if (ip_mc_check_igmp(skb, NULL) < 0)
+	if (ip_mc_check_igmp(skb) < 0)
 		return false;
 
 	switch (igmp_hdr(skb)->type) {
@@ -741,7 +741,7 @@  static int batadv_mcast_forw_mode_check_ipv4(struct batadv_priv *bat_priv,
  */
 static bool batadv_mcast_is_report_ipv6(struct sk_buff *skb)
 {
-	if (ipv6_mc_check_mld(skb, NULL) < 0)
+	if (ipv6_mc_check_mld(skb) < 0)
 		return false;
 
 	switch (icmp6_hdr(skb)->icmp6_type) {