[PATCH 1/5] NETFILTER ctnetlink: use ENOMEM instead of -1

[PATCH 1/5] NETFILTER ctnetlink: use ENOMEM instead of -1

[Harald Welte]

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

Hi Dave,

I think this can still go into 2.6.14, since it is very obvious and
actually fixes a bug.  Please apply, thanks.

[NETFILTER] ctnetlink: use ENOMEM instead of -1 (-ENOPERM)

Fix the return value of NFA_PUT on fail. Now it returns -ENOMEM
instead of -1 (-ENOPERM) since NFA_PUT only fails if there's no
enough space in the netlink message for the new attribute that
will be added. Thanks to Patrick McHardy for spotting this.

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

---
commit c12716a196b7d9fc9fc64a8c7982c2564c3c998f
tree 2d7bd287926008f17bbf0df3431f6642f6bcc041
parent 4714eea267d67e65aabb6518cbfa81b8c45c2147
author Harald Welte <laforge@netfilter.org> Fri, 07 Oct 2005 23:34:46 +0200
committer Harald Welte <laforge@netfilter.org> Fri, 07 Oct 2005 23:34:46 +0200

 net/ipv4/netfilter/ip_conntrack_netlink.c    |   42 +++++++++++++-------------
 net/ipv4/netfilter/ip_conntrack_proto_icmp.c |    2 +
 net/ipv4/netfilter/ip_conntrack_proto_tcp.c  |    2 +
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/net/ipv4/netfilter/ip_conntrack_netlink.c b/net/ipv4/netfilter/ip_conntrack_netlink.c
--- a/net/ipv4/netfilter/ip_conntrack_netlink.c
+++ b/net/ipv4/netfilter/ip_conntrack_netlink.c
@@ -68,7 +68,7 @@ ctnetlink_dump_tuples_proto(struct sk_bu
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 static inline int
@@ -89,7 +89,7 @@ ctnetlink_dump_tuples(struct sk_buff *sk
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 static inline int
@@ -100,7 +100,7 @@ ctnetlink_dump_status(struct sk_buff *sk
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 static inline int
@@ -118,7 +118,7 @@ ctnetlink_dump_timeout(struct sk_buff *s
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 static inline int
@@ -143,7 +143,7 @@ ctnetlink_dump_protoinfo(struct sk_buff 
 	return ret;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 static inline int
@@ -165,7 +165,7 @@ ctnetlink_dump_helpinfo(struct sk_buff *
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 #ifdef CONFIG_IP_NF_CT_ACCT
@@ -188,7 +188,7 @@ ctnetlink_dump_counters(struct sk_buff *
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 #else
 #define ctnetlink_dump_counters(a, b, c) (0)
@@ -204,7 +204,7 @@ ctnetlink_dump_mark(struct sk_buff *skb,
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 #else
 #define ctnetlink_dump_mark(a, b) (0)
@@ -218,7 +218,7 @@ ctnetlink_dump_id(struct sk_buff *skb, c
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 static inline int
@@ -230,7 +230,7 @@ ctnetlink_dump_use(struct sk_buff *skb, 
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 #define tuple(ct, dir) (&(ct)->tuplehash[dir].tuple)
@@ -283,7 +283,7 @@ ctnetlink_fill_info(struct sk_buff *skb,
 nlmsg_failure:
 nfattr_failure:
 	skb_trim(skb, b - skb->data);
-	return -1;
+	return -ENOMEM;
 }
 
 #ifdef CONFIG_IP_NF_CONNTRACK_EVENTS
@@ -499,7 +499,7 @@ ctnetlink_parse_tuple_ip(struct nfattr *
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 static const int cta_min_proto[CTA_PROTO_MAX] = {
@@ -541,7 +541,7 @@ ctnetlink_parse_tuple_proto(struct nfatt
 	return ret;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 static inline int
@@ -585,7 +585,7 @@ ctnetlink_parse_tuple(struct nfattr *cda
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 #ifdef CONFIG_IP_NF_NAT_NEEDED
@@ -628,7 +628,7 @@ static int ctnetlink_parse_nat_proto(str
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 static inline int
@@ -667,7 +667,7 @@ ctnetlink_parse_nat(struct nfattr *cda[]
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 #endif
 
@@ -689,7 +689,7 @@ ctnetlink_parse_help(struct nfattr *attr
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 static int
@@ -827,7 +827,7 @@ ctnetlink_get_conntrack(struct sock *ctn
 out:
 	if (skb2)
 		kfree_skb(skb2);
-	return -1;
+	return err;
 }
 
 static inline int
@@ -1093,7 +1093,7 @@ ctnetlink_exp_dump_tuple(struct sk_buff 
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }			
 
 static inline int
@@ -1119,7 +1119,7 @@ ctnetlink_exp_dump_expect(struct sk_buff
 	return 0;
 	
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 static int
@@ -1152,7 +1152,7 @@ ctnetlink_exp_fill_info(struct sk_buff *
 nlmsg_failure:
 nfattr_failure:
 	skb_trim(skb, b - skb->data);
-	return -1;
+	return -ENOMEM;
 }
 
 #ifdef CONFIG_IP_NF_CONNTRACK_EVENTS
diff --git a/net/ipv4/netfilter/ip_conntrack_proto_icmp.c b/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
--- a/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
+++ b/net/ipv4/netfilter/ip_conntrack_proto_icmp.c
@@ -289,7 +289,7 @@ static int icmp_tuple_to_nfattr(struct s
 	return 0;
 
 nfattr_failure:
-	return -1;
+	return -ENOMEM;
 }
 
 static int icmp_nfattr_to_tuple(struct nfattr *tb[],
diff --git a/net/ipv4/netfilter/ip_conntrack_proto_tcp.c b/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
--- a/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
+++ b/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
@@ -350,7 +350,7 @@ static int tcp_to_nfattr(struct sk_buff 
 
 nfattr_failure:
 	read_unlock_bh(&tcp_lock);
-	return -1;
+	return -ENOMEM;
 }
 #endif
 
-- 
- Harald Welte <laforge@gnumonks.org>          	        http://gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

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

Re: [PATCH 1/5] NETFILTER ctnetlink: use ENOMEM instead of -1

[Patrick McHardy]

Harald Welte wrote:
> [NETFILTER] ctnetlink: use ENOMEM instead of -1 (-ENOPERM)
> 
> Fix the return value of NFA_PUT on fail. Now it returns -ENOMEM
> instead of -1 (-ENOPERM) since NFA_PUT only fails if there's no
> enough space in the netlink message for the new attribute that
> will be added. Thanks to Patrick McHardy for spotting this.

Actually this is not what I meant. On paths were NFA_PUT is used,
returning -1 is fine because the caller just checks for <0 to
notice an error and the error is not returned to userspace or
something. -ENOMEM is obviously wrong since there is no memory
allocation involved. What I meant was to not return -1 on
the receive path, where NFA_PUT isn't used, but to propagate
the real error from the function call before (I don't remember
which one it was).

Dave, please drop this patch.

Re: [PATCH 1/5] NETFILTER ctnetlink: use ENOMEM instead of -1

[David S. Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Sat, 08 Oct 2005 17:42:44 +0200

> Dave, please drop this patch.

I'll kill it from my tree, thanks.

Re: [PATCH 1/5] NETFILTER ctnetlink: use ENOMEM instead of -1

[Harald Welte]

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

On Sun, Oct 09, 2005 at 09:52:22PM -0700, David S. Miller wrote:
> From: Patrick McHardy <kaber@trash.net>
> Date: Sat, 08 Oct 2005 17:42:44 +0200
> 
> > Dave, please drop this patch.
> 
> I'll kill it from my tree, thanks.

sorry for letting this through, I didn't really look at it in detail,
as it was Pablo who submitted it, and allegedly by a suggestion from
Patrick - and on a superficial level it seemed right :(

-- 
- 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 --]

Re: [PATCH 1/5] NETFILTER ctnetlink: use ENOMEM instead of -1

[David S. Miller]

From: Harald Welte <laforge@gnumonks.org>
Date: Sat, 8 Oct 2005 00:19:48 +0200

> [NETFILTER] ctnetlink: use ENOMEM instead of -1 (-ENOPERM)
> 
> Fix the return value of NFA_PUT on fail. Now it returns -ENOMEM
> instead of -1 (-ENOPERM) since NFA_PUT only fails if there's no
> enough space in the netlink message for the new attribute that
> will be added. Thanks to Patrick McHardy for spotting this.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Signed-off-by: Harald Welte <laforge@netfilter.org>

Applied for 2.6.14, thanks.