netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] netfilter fixes for net-next
@ 2013-04-25  0:22 Pablo Neira Ayuso
  2013-04-25  0:22 ` [PATCH 01/10] ipvs: properly dereference dest_dst in ip_vs_forget_dev Pablo Neira Ayuso
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2013-04-25  0:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Hi David,

The following patchset contains fixes for recently applied
Netfilter/IPVS updates to the net-next tree, most relevantly
they are:

* Fix sparse warnings introduced in the RCU conversion, from
  Julian Anastasov.

* Fix wrong endianness in the size field of IPVS sync messages,
  from Simon Horman.

* Fix missing if checking in nf_xfrm_me_harder, from Dan Carpenter.

* Fix off by one access in the IPVS SCTP tracking code, again from
  Dan Carpenter.

The following changes since commit 3e3251b3f289528732edab386ddf73ac428359b7:

  net: sctp: minor: remove dead code from sctp_packet (2013-04-22 16:25:21 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master

for you to fetch changes up to e7e6f6300faaafe05380ca5455b99c2a8f1f51a0:

  netfilter: nf_nat: missing condition in nf_xfrm_me_harder() (2013-04-25 01:58:16 +0200)

----------------------------------------------------------------
Dan Carpenter (2):
      ipvs: off by one in set_sctp_state()
      netfilter: nf_nat: missing condition in nf_xfrm_me_harder()

Julian Anastasov (5):
      ipvs: properly dereference dest_dst in ip_vs_forget_dev
      ipvs: fix sparse warnings for ip_vs_conn listing
      ipvs: fix the remaining sparse warnings in ip_vs_ctl.c
      ipvs: fix sparse warnings in lblc and lblcr
      ipvs: fix sparse warnings for some parameters

Simon Horman (3):
      ipvs: Avoid shadowing net variable in ip_vs_leave()
      ipvs: Use min3() in ip_vs_dbg_callid()
      ipvs: Use network byte order for sync message size

 include/net/ip_vs.h                   |    8 ++---
 include/uapi/linux/ip_vs.h            |    4 +--
 net/netfilter/ipvs/ip_vs_conn.c       |   14 ++++-----
 net/netfilter/ipvs/ip_vs_core.c       |    7 +++--
 net/netfilter/ipvs/ip_vs_ctl.c        |   55 ++++++++++++++++++++-------------
 net/netfilter/ipvs/ip_vs_lblc.c       |    2 +-
 net/netfilter/ipvs/ip_vs_lblcr.c      |    2 +-
 net/netfilter/ipvs/ip_vs_pe_sip.c     |    3 +-
 net/netfilter/ipvs/ip_vs_proto_sctp.c |    2 +-
 net/netfilter/ipvs/ip_vs_sync.c       |   21 +++++--------
 net/netfilter/nf_nat_core.c           |    1 +
 11 files changed, 64 insertions(+), 55 deletions(-)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 01/10] ipvs: properly dereference dest_dst in ip_vs_forget_dev
  2013-04-25  0:22 [PATCH 00/10] netfilter fixes for net-next Pablo Neira Ayuso
@ 2013-04-25  0:22 ` Pablo Neira Ayuso
  2013-04-25  0:22 ` [PATCH 02/10] ipvs: fix sparse warnings for ip_vs_conn listing Pablo Neira Ayuso
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2013-04-25  0:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Julian Anastasov <ja@ssi.bg>

Use rcu_dereference_protected to resolve
sparse warning, found by kbuild test robot:

net/netfilter/ipvs/ip_vs_ctl.c:1464:35: warning: dereference of
noderef expression

Problem from commit 026ace060dfe29
("ipvs: optimize dst usage for real server")

Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_ctl.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 9e4074c..5a65444 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1460,8 +1460,11 @@ void ip_vs_service_net_cleanup(struct net *net)
 static inline void
 ip_vs_forget_dev(struct ip_vs_dest *dest, struct net_device *dev)
 {
+	struct ip_vs_dest_dst *dest_dst;
+
 	spin_lock_bh(&dest->dst_lock);
-	if (dest->dest_dst && dest->dest_dst->dst_cache->dev == dev) {
+	dest_dst = rcu_dereference_protected(dest->dest_dst, 1);
+	if (dest_dst && dest_dst->dst_cache->dev == dev) {
 		IP_VS_DBG_BUF(3, "Reset dev:%s dest %s:%u ,dest->refcnt=%d\n",
 			      dev->name,
 			      IP_VS_DBG_ADDR(dest->af, &dest->addr),
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 02/10] ipvs: fix sparse warnings for ip_vs_conn listing
  2013-04-25  0:22 [PATCH 00/10] netfilter fixes for net-next Pablo Neira Ayuso
  2013-04-25  0:22 ` [PATCH 01/10] ipvs: properly dereference dest_dst in ip_vs_forget_dev Pablo Neira Ayuso
@ 2013-04-25  0:22 ` Pablo Neira Ayuso
  2013-04-25  0:22 ` [PATCH 03/10] ipvs: fix the remaining sparse warnings in ip_vs_ctl.c Pablo Neira Ayuso
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2013-04-25  0:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Julian Anastasov <ja@ssi.bg>

kbuild test robot reports for sparse warnings
in commit 088339a57d6042 ("ipvs: convert connection locking"):

net/netfilter/ipvs/ip_vs_conn.c:962:13: warning: context imbalance
	in 'ip_vs_conn_array' - wrong count at exit
include/linux/rcupdate.h:326:30: warning: context imbalance in
	'ip_vs_conn_seq_next' - unexpected unlock
include/linux/rcupdate.h:326:30: warning: context imbalance in
	'ip_vs_conn_seq_stop' - unexpected unlock

Fix it by running ip_vs_conn_array under RCU lock
to avoid conditional locking and by adding proper RCU
annotations.

Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_conn.c |   14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index de64758..a083bda 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -966,7 +966,6 @@ static void *ip_vs_conn_array(struct seq_file *seq, loff_t pos)
 	struct ip_vs_iter_state *iter = seq->private;
 
 	for (idx = 0; idx < ip_vs_conn_tab_size; idx++) {
-		rcu_read_lock();
 		hlist_for_each_entry_rcu(cp, &ip_vs_conn_tab[idx], c_list) {
 			/* __ip_vs_conn_get() is not needed by
 			 * ip_vs_conn_seq_show and ip_vs_conn_sync_seq_show
@@ -977,16 +976,19 @@ static void *ip_vs_conn_array(struct seq_file *seq, loff_t pos)
 			}
 		}
 		rcu_read_unlock();
+		rcu_read_lock();
 	}
 
 	return NULL;
 }
 
 static void *ip_vs_conn_seq_start(struct seq_file *seq, loff_t *pos)
+	__acquires(RCU)
 {
 	struct ip_vs_iter_state *iter = seq->private;
 
 	iter->l = NULL;
+	rcu_read_lock();
 	return *pos ? ip_vs_conn_array(seq, *pos - 1) :SEQ_START_TOKEN;
 }
 
@@ -1006,28 +1008,24 @@ static void *ip_vs_conn_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 	e = rcu_dereference(hlist_next_rcu(&cp->c_list));
 	if (e)
 		return hlist_entry(e, struct ip_vs_conn, c_list);
-	rcu_read_unlock();
 
 	idx = l - ip_vs_conn_tab;
 	while (++idx < ip_vs_conn_tab_size) {
-		rcu_read_lock();
 		hlist_for_each_entry_rcu(cp, &ip_vs_conn_tab[idx], c_list) {
 			iter->l = &ip_vs_conn_tab[idx];
 			return cp;
 		}
 		rcu_read_unlock();
+		rcu_read_lock();
 	}
 	iter->l = NULL;
 	return NULL;
 }
 
 static void ip_vs_conn_seq_stop(struct seq_file *seq, void *v)
+	__releases(RCU)
 {
-	struct ip_vs_iter_state *iter = seq->private;
-	struct hlist_head *l = iter->l;
-
-	if (l)
-		rcu_read_unlock();
+	rcu_read_unlock();
 }
 
 static int ip_vs_conn_seq_show(struct seq_file *seq, void *v)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 03/10] ipvs: fix the remaining sparse warnings in ip_vs_ctl.c
  2013-04-25  0:22 [PATCH 00/10] netfilter fixes for net-next Pablo Neira Ayuso
  2013-04-25  0:22 ` [PATCH 01/10] ipvs: properly dereference dest_dst in ip_vs_forget_dev Pablo Neira Ayuso
  2013-04-25  0:22 ` [PATCH 02/10] ipvs: fix sparse warnings for ip_vs_conn listing Pablo Neira Ayuso
@ 2013-04-25  0:22 ` Pablo Neira Ayuso
  2013-04-25  0:22 ` [PATCH 04/10] ipvs: fix sparse warnings in lblc and lblcr Pablo Neira Ayuso
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2013-04-25  0:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Julian Anastasov <ja@ssi.bg>

- RCU annotations for ip_vs_info_seq_start and _stop
- __percpu for cpustats
- properly dereference svc->pe in ip_vs_genl_fill_service

Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_ctl.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 5a65444..64075a7 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1937,8 +1937,8 @@ static struct ip_vs_service *ip_vs_info_array(struct seq_file *seq, loff_t pos)
 }
 
 static void *ip_vs_info_seq_start(struct seq_file *seq, loff_t *pos)
+	__acquires(RCU)
 {
-
 	rcu_read_lock();
 	return *pos ? ip_vs_info_array(seq, *pos - 1) : SEQ_START_TOKEN;
 }
@@ -1993,6 +1993,7 @@ static void *ip_vs_info_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 }
 
 static void ip_vs_info_seq_stop(struct seq_file *seq, void *v)
+	__releases(RCU)
 {
 	rcu_read_unlock();
 }
@@ -2137,7 +2138,7 @@ static int ip_vs_stats_percpu_show(struct seq_file *seq, void *v)
 {
 	struct net *net = seq_file_single_net(seq);
 	struct ip_vs_stats *tot_stats = &net_ipvs(net)->tot_stats;
-	struct ip_vs_cpu_stats *cpustats = tot_stats->cpustats;
+	struct ip_vs_cpu_stats __percpu *cpustats = tot_stats->cpustats;
 	struct ip_vs_stats_user rates;
 	int i;
 
@@ -2874,6 +2875,7 @@ static int ip_vs_genl_fill_service(struct sk_buff *skb,
 				   struct ip_vs_service *svc)
 {
 	struct ip_vs_scheduler *sched;
+	struct ip_vs_pe *pe;
 	struct nlattr *nl_service;
 	struct ip_vs_flags flags = { .flags = svc->flags,
 				     .mask = ~0 };
@@ -2895,9 +2897,9 @@ static int ip_vs_genl_fill_service(struct sk_buff *skb,
 	}
 
 	sched = rcu_dereference_protected(svc->scheduler, 1);
+	pe = rcu_dereference_protected(svc->pe, 1);
 	if (nla_put_string(skb, IPVS_SVC_ATTR_SCHED_NAME, sched->name) ||
-	    (svc->pe &&
-	     nla_put_string(skb, IPVS_SVC_ATTR_PE_NAME, svc->pe->name)) ||
+	    (pe && nla_put_string(skb, IPVS_SVC_ATTR_PE_NAME, pe->name)) ||
 	    nla_put(skb, IPVS_SVC_ATTR_FLAGS, sizeof(flags), &flags) ||
 	    nla_put_u32(skb, IPVS_SVC_ATTR_TIMEOUT, svc->timeout / HZ) ||
 	    nla_put_u32(skb, IPVS_SVC_ATTR_NETMASK, svc->netmask))
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 04/10] ipvs: fix sparse warnings in lblc and lblcr
  2013-04-25  0:22 [PATCH 00/10] netfilter fixes for net-next Pablo Neira Ayuso
                   ` (2 preceding siblings ...)
  2013-04-25  0:22 ` [PATCH 03/10] ipvs: fix the remaining sparse warnings in ip_vs_ctl.c Pablo Neira Ayuso
@ 2013-04-25  0:22 ` Pablo Neira Ayuso
  2013-04-25  0:22 ` [PATCH 05/10] ipvs: fix sparse warnings for some parameters Pablo Neira Ayuso
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2013-04-25  0:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Julian Anastasov <ja@ssi.bg>

kbuild test robot reports for sparse warnings in
commits c2a4ffb70eef39 ("ipvs: convert lblc scheduler to rcu")
and c5549571f975ab ("ipvs: convert lblcr scheduler to rcu").

Fix it by removing extra __rcu annotation.

Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_lblc.c  |    2 +-
 net/netfilter/ipvs/ip_vs_lblcr.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_lblc.c b/net/netfilter/ipvs/ip_vs_lblc.c
index b2cc252..5ea26bd 100644
--- a/net/netfilter/ipvs/ip_vs_lblc.c
+++ b/net/netfilter/ipvs/ip_vs_lblc.c
@@ -104,7 +104,7 @@ struct ip_vs_lblc_entry {
  */
 struct ip_vs_lblc_table {
 	struct rcu_head		rcu_head;
-	struct hlist_head __rcu bucket[IP_VS_LBLC_TAB_SIZE];  /* hash bucket */
+	struct hlist_head	bucket[IP_VS_LBLC_TAB_SIZE];  /* hash bucket */
 	struct timer_list       periodic_timer; /* collect stale entries */
 	atomic_t                entries;        /* number of entries */
 	int                     max_size;       /* maximum size of entries */
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index feb9656..50123c2 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -284,7 +284,7 @@ struct ip_vs_lblcr_entry {
  */
 struct ip_vs_lblcr_table {
 	struct rcu_head		rcu_head;
-	struct hlist_head __rcu bucket[IP_VS_LBLCR_TAB_SIZE];  /* hash bucket */
+	struct hlist_head	bucket[IP_VS_LBLCR_TAB_SIZE];  /* hash bucket */
 	atomic_t                entries;        /* number of entries */
 	int                     max_size;       /* maximum size of entries */
 	struct timer_list       periodic_timer; /* collect stale entries */
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 05/10] ipvs: fix sparse warnings for some parameters
  2013-04-25  0:22 [PATCH 00/10] netfilter fixes for net-next Pablo Neira Ayuso
                   ` (3 preceding siblings ...)
  2013-04-25  0:22 ` [PATCH 04/10] ipvs: fix sparse warnings in lblc and lblcr Pablo Neira Ayuso
@ 2013-04-25  0:22 ` Pablo Neira Ayuso
  2013-04-25  0:22 ` [PATCH 06/10] ipvs: Avoid shadowing net variable in ip_vs_leave() Pablo Neira Ayuso
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2013-04-25  0:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Julian Anastasov <ja@ssi.bg>

Some service fields are in network order:

- netmask: used once in network order and also as prefix len for IPv6
- port

Other parameters are in host order:

- struct ip_vs_flags: flags and mask moved between user and kernel only
- sync state: moved between user and kernel only
- syncid: sent over network as single octet

Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 include/net/ip_vs.h             |    8 ++++----
 include/uapi/linux/ip_vs.h      |    4 ++--
 net/netfilter/ipvs/ip_vs_core.c |    3 ++-
 net/netfilter/ipvs/ip_vs_ctl.c  |   40 +++++++++++++++++++++++----------------
 4 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index f9f5b05..4c062cc 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -678,7 +678,7 @@ struct ip_vs_service_user_kern {
 	u16			af;
 	u16			protocol;
 	union nf_inet_addr	addr;		/* virtual ip address */
-	u16			port;
+	__be16			port;
 	u32			fwmark;		/* firwall mark of service */
 
 	/* virtual service options */
@@ -686,14 +686,14 @@ struct ip_vs_service_user_kern {
 	char			*pe_name;
 	unsigned int		flags;		/* virtual service flags */
 	unsigned int		timeout;	/* persistent timeout in sec */
-	u32			netmask;	/* persistent netmask */
+	__be32			netmask;	/* persistent netmask or plen */
 };
 
 
 struct ip_vs_dest_user_kern {
 	/* destination server address */
 	union nf_inet_addr	addr;
-	u16			port;
+	__be16			port;
 
 	/* real server options */
 	unsigned int		conn_flags;	/* connection flags */
@@ -721,7 +721,7 @@ struct ip_vs_service {
 	__u32                   fwmark;   /* firewall mark of the service */
 	unsigned int		flags;	  /* service status flags */
 	unsigned int		timeout;  /* persistent timeout in ticks */
-	__be32			netmask;  /* grouping granularity */
+	__be32			netmask;  /* grouping granularity, mask/plen */
 	struct net		*net;
 
 	struct list_head	destinations;  /* real server d-linked list */
diff --git a/include/uapi/linux/ip_vs.h b/include/uapi/linux/ip_vs.h
index 8a2d438..a245377 100644
--- a/include/uapi/linux/ip_vs.h
+++ b/include/uapi/linux/ip_vs.h
@@ -280,8 +280,8 @@ struct ip_vs_daemon_user {
 #define IPVS_GENL_VERSION	0x1
 
 struct ip_vs_flags {
-	__be32 flags;
-	__be32 mask;
+	__u32 flags;
+	__u32 mask;
 };
 
 /* Generic Netlink command attributes */
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index f26fe33..a0d7bd3 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -235,7 +235,8 @@ ip_vs_sched_persist(struct ip_vs_service *svc,
 	/* Mask saddr with the netmask to adjust template granularity */
 #ifdef CONFIG_IP_VS_IPV6
 	if (svc->af == AF_INET6)
-		ipv6_addr_prefix(&snet.in6, &iph->saddr.in6, svc->netmask);
+		ipv6_addr_prefix(&snet.in6, &iph->saddr.in6,
+				 (__force __u32) svc->netmask);
 	else
 #endif
 		snet.ip = iph->saddr.ip & svc->netmask;
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 64075a7..5b142fb 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1164,9 +1164,13 @@ ip_vs_add_service(struct net *net, struct ip_vs_service_user_kern *u,
 	}
 
 #ifdef CONFIG_IP_VS_IPV6
-	if (u->af == AF_INET6 && (u->netmask < 1 || u->netmask > 128)) {
-		ret = -EINVAL;
-		goto out_err;
+	if (u->af == AF_INET6) {
+		__u32 plen = (__force __u32) u->netmask;
+
+		if (plen < 1 || plen > 128) {
+			ret = -EINVAL;
+			goto out_err;
+		}
 	}
 #endif
 
@@ -1277,9 +1281,13 @@ ip_vs_edit_service(struct ip_vs_service *svc, struct ip_vs_service_user_kern *u)
 	}
 
 #ifdef CONFIG_IP_VS_IPV6
-	if (u->af == AF_INET6 && (u->netmask < 1 || u->netmask > 128)) {
-		ret = -EINVAL;
-		goto out;
+	if (u->af == AF_INET6) {
+		__u32 plen = (__force __u32) u->netmask;
+
+		if (plen < 1 || plen > 128) {
+			ret = -EINVAL;
+			goto out;
+		}
 	}
 #endif
 
@@ -2892,7 +2900,7 @@ static int ip_vs_genl_fill_service(struct sk_buff *skb,
 	} else {
 		if (nla_put_u16(skb, IPVS_SVC_ATTR_PROTOCOL, svc->protocol) ||
 		    nla_put(skb, IPVS_SVC_ATTR_ADDR, sizeof(svc->addr), &svc->addr) ||
-		    nla_put_u16(skb, IPVS_SVC_ATTR_PORT, svc->port))
+		    nla_put_be16(skb, IPVS_SVC_ATTR_PORT, svc->port))
 			goto nla_put_failure;
 	}
 
@@ -2902,7 +2910,7 @@ static int ip_vs_genl_fill_service(struct sk_buff *skb,
 	    (pe && nla_put_string(skb, IPVS_SVC_ATTR_PE_NAME, pe->name)) ||
 	    nla_put(skb, IPVS_SVC_ATTR_FLAGS, sizeof(flags), &flags) ||
 	    nla_put_u32(skb, IPVS_SVC_ATTR_TIMEOUT, svc->timeout / HZ) ||
-	    nla_put_u32(skb, IPVS_SVC_ATTR_NETMASK, svc->netmask))
+	    nla_put_be32(skb, IPVS_SVC_ATTR_NETMASK, svc->netmask))
 		goto nla_put_failure;
 	if (ip_vs_genl_fill_stats(skb, IPVS_SVC_ATTR_STATS, &svc->stats))
 		goto nla_put_failure;
@@ -3015,7 +3023,7 @@ static int ip_vs_genl_parse_service(struct net *net,
 	} else {
 		usvc->protocol = nla_get_u16(nla_protocol);
 		nla_memcpy(&usvc->addr, nla_addr, sizeof(usvc->addr));
-		usvc->port = nla_get_u16(nla_port);
+		usvc->port = nla_get_be16(nla_port);
 		usvc->fwmark = 0;
 	}
 
@@ -3055,7 +3063,7 @@ static int ip_vs_genl_parse_service(struct net *net,
 		usvc->sched_name = nla_data(nla_sched);
 		usvc->pe_name = nla_pe ? nla_data(nla_pe) : NULL;
 		usvc->timeout = nla_get_u32(nla_timeout);
-		usvc->netmask = nla_get_u32(nla_netmask);
+		usvc->netmask = nla_get_be32(nla_netmask);
 	}
 
 	return 0;
@@ -3081,7 +3089,7 @@ static int ip_vs_genl_fill_dest(struct sk_buff *skb, struct ip_vs_dest *dest)
 		return -EMSGSIZE;
 
 	if (nla_put(skb, IPVS_DEST_ATTR_ADDR, sizeof(dest->addr), &dest->addr) ||
-	    nla_put_u16(skb, IPVS_DEST_ATTR_PORT, dest->port) ||
+	    nla_put_be16(skb, IPVS_DEST_ATTR_PORT, dest->port) ||
 	    nla_put_u32(skb, IPVS_DEST_ATTR_FWD_METHOD,
 			(atomic_read(&dest->conn_flags) &
 			 IP_VS_CONN_F_FWD_MASK)) ||
@@ -3190,7 +3198,7 @@ static int ip_vs_genl_parse_dest(struct ip_vs_dest_user_kern *udest,
 	memset(udest, 0, sizeof(*udest));
 
 	nla_memcpy(&udest->addr, nla_addr, sizeof(udest->addr));
-	udest->port = nla_get_u16(nla_port);
+	udest->port = nla_get_be16(nla_port);
 
 	/* If a full entry was requested, check for the additional fields */
 	if (full_entry) {
@@ -3215,8 +3223,8 @@ static int ip_vs_genl_parse_dest(struct ip_vs_dest_user_kern *udest,
 	return 0;
 }
 
-static int ip_vs_genl_fill_daemon(struct sk_buff *skb, __be32 state,
-				  const char *mcast_ifn, __be32 syncid)
+static int ip_vs_genl_fill_daemon(struct sk_buff *skb, __u32 state,
+				  const char *mcast_ifn, __u32 syncid)
 {
 	struct nlattr *nl_daemon;
 
@@ -3237,8 +3245,8 @@ nla_put_failure:
 	return -EMSGSIZE;
 }
 
-static int ip_vs_genl_dump_daemon(struct sk_buff *skb, __be32 state,
-				  const char *mcast_ifn, __be32 syncid,
+static int ip_vs_genl_dump_daemon(struct sk_buff *skb, __u32 state,
+				  const char *mcast_ifn, __u32 syncid,
 				  struct netlink_callback *cb)
 {
 	void *hdr;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 06/10] ipvs: Avoid shadowing net variable in ip_vs_leave()
  2013-04-25  0:22 [PATCH 00/10] netfilter fixes for net-next Pablo Neira Ayuso
                   ` (4 preceding siblings ...)
  2013-04-25  0:22 ` [PATCH 05/10] ipvs: fix sparse warnings for some parameters Pablo Neira Ayuso
@ 2013-04-25  0:22 ` Pablo Neira Ayuso
  2013-04-25  0:22 ` [PATCH 07/10] ipvs: Use min3() in ip_vs_dbg_callid() Pablo Neira Ayuso
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2013-04-25  0:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Simon Horman <horms@verge.net.au>

Flagged by sparse.
Compile and sparse tested only.

Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_core.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index a0d7bd3..085b588 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -584,9 +584,9 @@ int ip_vs_leave(struct ip_vs_service *svc, struct sk_buff *skb,
 #ifdef CONFIG_IP_VS_IPV6
 	if (svc->af == AF_INET6) {
 		if (!skb->dev) {
-			struct net *net = dev_net(skb_dst(skb)->dev);
+			struct net *net_ = dev_net(skb_dst(skb)->dev);
 
-			skb->dev = net->loopback_dev;
+			skb->dev = net_->loopback_dev;
 		}
 		icmpv6_send(skb, ICMPV6_DEST_UNREACH, ICMPV6_PORT_UNREACH, 0);
 	} else
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 07/10] ipvs: Use min3() in ip_vs_dbg_callid()
  2013-04-25  0:22 [PATCH 00/10] netfilter fixes for net-next Pablo Neira Ayuso
                   ` (5 preceding siblings ...)
  2013-04-25  0:22 ` [PATCH 06/10] ipvs: Avoid shadowing net variable in ip_vs_leave() Pablo Neira Ayuso
@ 2013-04-25  0:22 ` Pablo Neira Ayuso
  2013-04-25  0:22 ` [PATCH 08/10] ipvs: off by one in set_sctp_state() Pablo Neira Ayuso
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2013-04-25  0:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Simon Horman <horms@verge.net.au>

There are two motivations for this:

1. It improves readability to my eyes
2. Using nested min() calls results in a shadowed _min1 variable,
   which is a bit untidy. Sparse complained about this.

I have also replaced (size_t)64 with a variable of type size_t and value 64.
This also improves readability to my eyes.

Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_pe_sip.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_pe_sip.c b/net/netfilter/ipvs/ip_vs_pe_sip.c
index 00cc024..9a8f421 100644
--- a/net/netfilter/ipvs/ip_vs_pe_sip.c
+++ b/net/netfilter/ipvs/ip_vs_pe_sip.c
@@ -13,7 +13,8 @@ static const char *ip_vs_dbg_callid(char *buf, size_t buf_len,
 				    const char *callid, size_t callid_len,
 				    int *idx)
 {
-	size_t len = min(min(callid_len, (size_t)64), buf_len - *idx - 1);
+	size_t max_len = 64;
+	size_t len = min3(max_len, callid_len, buf_len - *idx - 1);
 	memcpy(buf + *idx, callid, len);
 	buf[*idx+len] = '\0';
 	*idx += len + 1;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 08/10] ipvs: off by one in set_sctp_state()
  2013-04-25  0:22 [PATCH 00/10] netfilter fixes for net-next Pablo Neira Ayuso
                   ` (6 preceding siblings ...)
  2013-04-25  0:22 ` [PATCH 07/10] ipvs: Use min3() in ip_vs_dbg_callid() Pablo Neira Ayuso
@ 2013-04-25  0:22 ` Pablo Neira Ayuso
  2013-04-25  0:22 ` [PATCH 09/10] ipvs: Use network byte order for sync message size Pablo Neira Ayuso
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2013-04-25  0:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Dan Carpenter <dan.carpenter@oracle.com>

The sctp_events[] come from sch->type in set_sctp_state().  They are
between 0-255 so that means we need 256 elements in the array.

I believe that because of how the code is aligned there is normally a
hole after sctp_events[] so this patch doesn't actually change anything.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_proto_sctp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index 6e14a7b..8646488 100644
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -208,7 +208,7 @@ enum ipvs_sctp_event_t {
 	IP_VS_SCTP_EVE_LAST
 };
 
-static enum ipvs_sctp_event_t sctp_events[255] = {
+static enum ipvs_sctp_event_t sctp_events[256] = {
 	IP_VS_SCTP_EVE_DATA_CLI,
 	IP_VS_SCTP_EVE_INIT_CLI,
 	IP_VS_SCTP_EVE_INIT_ACK_CLI,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 09/10] ipvs: Use network byte order for sync message size
  2013-04-25  0:22 [PATCH 00/10] netfilter fixes for net-next Pablo Neira Ayuso
                   ` (7 preceding siblings ...)
  2013-04-25  0:22 ` [PATCH 08/10] ipvs: off by one in set_sctp_state() Pablo Neira Ayuso
@ 2013-04-25  0:22 ` Pablo Neira Ayuso
  2013-04-25  0:22 ` [PATCH 10/10] netfilter: nf_nat: missing condition in nf_xfrm_me_harder() Pablo Neira Ayuso
  2013-04-25  4:54 ` [PATCH 00/10] netfilter fixes for net-next David Miller
  10 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2013-04-25  0:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Simon Horman <horms@verge.net.au>

struct ip_vs_sync_mesg and ip_vs_sync_mesg_v0 are both sent across the wire
and used internally to store IPVS synchronisation messages.

Up until now the scheme used has been to convert the size field
to network byte order before sending a message on the wire and
convert it to host byte order when sending a message.

This patch changes that scheme to always treat the field
as being network byte order. This seems appropriate as
the structure is sent across the wire. And by consistently
treating the field has network byte order it is now possible
to take advantage of sparse to flag any future miss-use.

Acked-by: Julian Anastasov <ja@ssi.bg>
Acked-by: Hans Schillstrom <hans@schillstrom.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_sync.c |   21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 8e57077..f6046d9 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -246,7 +246,7 @@ struct ip_vs_sync_thread_data {
 struct ip_vs_sync_mesg_v0 {
 	__u8                    nr_conns;
 	__u8                    syncid;
-	__u16                   size;
+	__be16                  size;
 
 	/* ip_vs_sync_conn entries start here */
 };
@@ -255,7 +255,7 @@ struct ip_vs_sync_mesg_v0 {
 struct ip_vs_sync_mesg {
 	__u8			reserved;	/* must be zero */
 	__u8			syncid;
-	__u16			size;
+	__be16			size;
 	__u8			nr_conns;
 	__s8			version;	/* SYNC_PROTO_VER  */
 	__u16			spare;
@@ -335,7 +335,7 @@ ip_vs_sync_buff_create(struct netns_ipvs *ipvs)
 	sb->mesg->reserved = 0;  /* old nr_conns i.e. must be zero now */
 	sb->mesg->version = SYNC_PROTO_VER;
 	sb->mesg->syncid = ipvs->master_syncid;
-	sb->mesg->size = sizeof(struct ip_vs_sync_mesg);
+	sb->mesg->size = htons(sizeof(struct ip_vs_sync_mesg));
 	sb->mesg->nr_conns = 0;
 	sb->mesg->spare = 0;
 	sb->head = (unsigned char *)sb->mesg + sizeof(struct ip_vs_sync_mesg);
@@ -418,7 +418,7 @@ ip_vs_sync_buff_create_v0(struct netns_ipvs *ipvs)
 	mesg = (struct ip_vs_sync_mesg_v0 *)sb->mesg;
 	mesg->nr_conns = 0;
 	mesg->syncid = ipvs->master_syncid;
-	mesg->size = sizeof(struct ip_vs_sync_mesg_v0);
+	mesg->size = htons(sizeof(struct ip_vs_sync_mesg_v0));
 	sb->head = (unsigned char *)mesg + sizeof(struct ip_vs_sync_mesg_v0);
 	sb->end = (unsigned char *)mesg + ipvs->send_mesg_maxlen;
 	sb->firstuse = jiffies;
@@ -582,7 +582,7 @@ static void ip_vs_sync_conn_v0(struct net *net, struct ip_vs_conn *cp,
 	}
 
 	m->nr_conns++;
-	m->size += len;
+	m->size = htons(ntohs(m->size) + len);
 	buff->head += len;
 
 	/* check if there is a space for next one */
@@ -693,7 +693,7 @@ sloop:
 
 	p = buff->head;
 	buff->head += pad + len;
-	m->size += pad + len;
+	m->size = htons(ntohs(m->size) + pad + len);
 	/* Add ev. padding from prev. sync_conn */
 	while (pad--)
 		*(p++) = 0;
@@ -1175,10 +1175,8 @@ static void ip_vs_process_message(struct net *net, __u8 *buffer,
 		IP_VS_DBG(2, "BACKUP, message header too short\n");
 		return;
 	}
-	/* Convert size back to host byte order */
-	m2->size = ntohs(m2->size);
 
-	if (buflen != m2->size) {
+	if (buflen != ntohs(m2->size)) {
 		IP_VS_DBG(2, "BACKUP, bogus message size\n");
 		return;
 	}
@@ -1544,10 +1542,7 @@ ip_vs_send_sync_msg(struct socket *sock, struct ip_vs_sync_mesg *msg)
 	int msize;
 	int ret;
 
-	msize = msg->size;
-
-	/* Put size in network byte order */
-	msg->size = htons(msg->size);
+	msize = ntohs(msg->size);
 
 	ret = ip_vs_send_async(sock, (char *)msg, msize);
 	if (ret >= 0 || ret == -EAGAIN)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 10/10] netfilter: nf_nat: missing condition in nf_xfrm_me_harder()
  2013-04-25  0:22 [PATCH 00/10] netfilter fixes for net-next Pablo Neira Ayuso
                   ` (8 preceding siblings ...)
  2013-04-25  0:22 ` [PATCH 09/10] ipvs: Use network byte order for sync message size Pablo Neira Ayuso
@ 2013-04-25  0:22 ` Pablo Neira Ayuso
  2013-04-25  4:54 ` [PATCH 00/10] netfilter fixes for net-next David Miller
  10 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2013-04-25  0:22 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Dan Carpenter <dan.carpenter@oracle.com>

This if statement was accidentally dropped in (aaa795a netfilter:
nat: propagate errors from xfrm_me_harder()) so now it returns
unconditionally.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_nat_core.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 346f871..cf1c731 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -90,6 +90,7 @@ int nf_xfrm_me_harder(struct sk_buff *skb, unsigned int family)
 	int err;
 
 	err = xfrm_decode_session(skb, &fl, family);
+	if (err < 0)
 		return err;
 
 	dst = skb_dst(skb);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 00/10] netfilter fixes for net-next
  2013-04-25  0:22 [PATCH 00/10] netfilter fixes for net-next Pablo Neira Ayuso
                   ` (9 preceding siblings ...)
  2013-04-25  0:22 ` [PATCH 10/10] netfilter: nf_nat: missing condition in nf_xfrm_me_harder() Pablo Neira Ayuso
@ 2013-04-25  4:54 ` David Miller
  10 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2013-04-25  4:54 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Thu, 25 Apr 2013 02:22:11 +0200

> The following patchset contains fixes for recently applied
> Netfilter/IPVS updates to the net-next tree, most relevantly
> they are:
> 
> * Fix sparse warnings introduced in the RCU conversion, from
>   Julian Anastasov.
> 
> * Fix wrong endianness in the size field of IPVS sync messages,
>   from Simon Horman.
> 
> * Fix missing if checking in nf_xfrm_me_harder, from Dan Carpenter.
> 
> * Fix off by one access in the IPVS SCTP tracking code, again from
>   Dan Carpenter.

Pulled, thanks Pablo.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-04-25  4:54 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-25  0:22 [PATCH 00/10] netfilter fixes for net-next Pablo Neira Ayuso
2013-04-25  0:22 ` [PATCH 01/10] ipvs: properly dereference dest_dst in ip_vs_forget_dev Pablo Neira Ayuso
2013-04-25  0:22 ` [PATCH 02/10] ipvs: fix sparse warnings for ip_vs_conn listing Pablo Neira Ayuso
2013-04-25  0:22 ` [PATCH 03/10] ipvs: fix the remaining sparse warnings in ip_vs_ctl.c Pablo Neira Ayuso
2013-04-25  0:22 ` [PATCH 04/10] ipvs: fix sparse warnings in lblc and lblcr Pablo Neira Ayuso
2013-04-25  0:22 ` [PATCH 05/10] ipvs: fix sparse warnings for some parameters Pablo Neira Ayuso
2013-04-25  0:22 ` [PATCH 06/10] ipvs: Avoid shadowing net variable in ip_vs_leave() Pablo Neira Ayuso
2013-04-25  0:22 ` [PATCH 07/10] ipvs: Use min3() in ip_vs_dbg_callid() Pablo Neira Ayuso
2013-04-25  0:22 ` [PATCH 08/10] ipvs: off by one in set_sctp_state() Pablo Neira Ayuso
2013-04-25  0:22 ` [PATCH 09/10] ipvs: Use network byte order for sync message size Pablo Neira Ayuso
2013-04-25  0:22 ` [PATCH 10/10] netfilter: nf_nat: missing condition in nf_xfrm_me_harder() Pablo Neira Ayuso
2013-04-25  4:54 ` [PATCH 00/10] netfilter fixes for net-next David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).