[PATCH 1/7] don't check nfattr_parse_nested return value

[PATCH 1/7] don't check nfattr_parse_nested return value

[Pablo Neira]

[-- Attachment #1: Type: text/plain, Size: 379 bytes --]

nfattr_parse_nested always returns success. So we don't need such
checking. It's unlikely that nfattr_parse would return something
different than 0 in the future, if that day comes for whatever reason we
could get the checking back.

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris

[-- Attachment #2: 04-parse_nested.patch --]
[-- Type: text/plain, Size: 4058 bytes --]

nfattr_parse_nested always returns success. So we don't need such checking.
It's unlikely that nfattr_parse would return something different than 0 in
the future, if that day comes for whatever reason we could get the checking
back.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

Index: net-2.6.git/net/ipv4/netfilter/ip_conntrack_netlink.c
===================================================================
--- net-2.6.git.orig/net/ipv4/netfilter/ip_conntrack_netlink.c	2005-11-04 17:45:17.000000000 +0100
+++ net-2.6.git/net/ipv4/netfilter/ip_conntrack_netlink.c	2005-11-04 18:31:07.000000000 +0100
@@ -482,9 +482,7 @@ ctnetlink_parse_tuple_ip(struct nfattr *
 
 	DEBUGP("entered %s\n", __FUNCTION__);
 
-	
-	if (nfattr_parse_nested(tb, CTA_IP_MAX, attr) < 0)
-		goto nfattr_failure;
+	nfattr_parse_nested(tb, CTA_IP_MAX, attr); 
 
 	if (nfattr_bad_size(tb, CTA_IP_MAX, cta_min_ip))
 		return -EINVAL;
@@ -500,9 +498,6 @@ ctnetlink_parse_tuple_ip(struct nfattr *
 	DEBUGP("leaving\n");
 
 	return 0;
-
-nfattr_failure:
-	return -1;
 }
 
 static const int cta_min_proto[CTA_PROTO_MAX] = {
@@ -524,8 +519,7 @@ ctnetlink_parse_tuple_proto(struct nfatt
 
 	DEBUGP("entered %s\n", __FUNCTION__);
 
-	if (nfattr_parse_nested(tb, CTA_PROTO_MAX, attr) < 0)
-		goto nfattr_failure;
+	nfattr_parse_nested(tb, CTA_PROTO_MAX, attr);
 
 	if (nfattr_bad_size(tb, CTA_PROTO_MAX, cta_min_proto))
 		return -EINVAL;
@@ -542,9 +536,6 @@ ctnetlink_parse_tuple_proto(struct nfatt
 	}
 	
 	return ret;
-
-nfattr_failure:
-	return -1;
 }
 
 static inline int
@@ -558,8 +549,7 @@ ctnetlink_parse_tuple(struct nfattr *cda
 
 	memset(tuple, 0, sizeof(*tuple));
 
-	if (nfattr_parse_nested(tb, CTA_TUPLE_MAX, cda[type-1]) < 0)
-		goto nfattr_failure;
+	nfattr_parse_nested(tb, CTA_TUPLE_MAX, cda[type-1]); 
 
 	if (!tb[CTA_TUPLE_IP-1])
 		return -EINVAL;
@@ -586,9 +576,6 @@ ctnetlink_parse_tuple(struct nfattr *cda
 	DEBUGP("leaving\n");
 
 	return 0;
-
-nfattr_failure:
-	return -1;
 }
 
 #ifdef CONFIG_IP_NF_NAT_NEEDED
@@ -606,11 +593,10 @@ static int ctnetlink_parse_nat_proto(str
 
 	DEBUGP("entered %s\n", __FUNCTION__);
 
-	if (nfattr_parse_nested(tb, CTA_PROTONAT_MAX, attr) < 0)
-		goto nfattr_failure;
+	nfattr_parse_nested(tb, CTA_PROTONAT_MAX, attr);
 
 	if (nfattr_bad_size(tb, CTA_PROTONAT_MAX, cta_min_protonat))
-		goto nfattr_failure;
+		return -EINVAL;
 
 	npt = ip_nat_proto_find_get(ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum);
 	if (!npt)
@@ -629,9 +615,6 @@ static int ctnetlink_parse_nat_proto(str
 
 	DEBUGP("leaving\n");
 	return 0;
-
-nfattr_failure:
-	return -1;
 }
 
 static inline int
@@ -645,8 +628,7 @@ ctnetlink_parse_nat(struct nfattr *cda[]
 
 	memset(range, 0, sizeof(*range));
 	
-	if (nfattr_parse_nested(tb, CTA_NAT_MAX, cda[CTA_NAT-1]) < 0)
-		goto nfattr_failure;
+	nfattr_parse_nested(tb, CTA_NAT_MAX, cda[CTA_NAT-1]);
 
 	if (tb[CTA_NAT_MINIP-1])
 		range->min_ip = *(u_int32_t *)NFA_DATA(tb[CTA_NAT_MINIP-1]);
@@ -668,9 +650,6 @@ ctnetlink_parse_nat(struct nfattr *cda[]
 
 	DEBUGP("leaving\n");
 	return 0;
-
-nfattr_failure:
-	return -1;
 }
 #endif
 
@@ -681,8 +660,7 @@ ctnetlink_parse_help(struct nfattr *attr
 
 	DEBUGP("entered %s\n", __FUNCTION__);
 
-	if (nfattr_parse_nested(tb, CTA_HELP_MAX, attr) < 0)
-		goto nfattr_failure;
+	nfattr_parse_nested(tb, CTA_HELP_MAX, attr); 
 
 	if (!tb[CTA_HELP_NAME-1])
 		return -EINVAL;
@@ -690,9 +668,6 @@ ctnetlink_parse_help(struct nfattr *attr
 	*helper_name = NFA_DATA(tb[CTA_HELP_NAME-1]);
 
 	return 0;
-
-nfattr_failure:
-	return -1;
 }
 
 static int
@@ -960,8 +935,7 @@ ctnetlink_change_protoinfo(struct ip_con
 	u_int16_t npt = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.protonum;
 	int err = 0;
 
-	if (nfattr_parse_nested(tb, CTA_PROTOINFO_MAX, attr) < 0)
-		goto nfattr_failure;
+	nfattr_parse_nested(tb, CTA_PROTOINFO_MAX, attr); 
 
 	proto = ip_conntrack_proto_find_get(npt);
 	if (!proto)
@@ -972,9 +946,6 @@ ctnetlink_change_protoinfo(struct ip_con
 	ip_conntrack_proto_put(proto); 
 
 	return err;
-
-nfattr_failure:
-	return -ENOMEM;
 }
 
 static int

Re: [PATCH 1/7] don't check nfattr_parse_nested return value

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 903 bytes --]

On Fri, Nov 04, 2005 at 07:00:16PM +0100, Pablo Neira wrote:
> nfattr_parse_nested always returns success. So we don't need such
> checking. It's unlikely that nfattr_parse would return something
> different than 0 in the future, if that day comes for whatever reason we
> could get the checking back.

if this is the case, then nfattr_parse() should become a void function.

also, you missed one occurrence of nfattr_parse_nested() in ip_conntrack_tcp.c

I'm fixing this up and apply a modified version of this patch.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]