Netdev List
 help / color / mirror / Atom feed
* [PATCH 3/4] netfilter: xtables: fix reentrancy
From: kaber @ 2011-03-20 15:32 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev
In-Reply-To: <1300635148-22648-1-git-send-email-kaber@trash.net>

From: Eric Dumazet <eric.dumazet@gmail.com>

commit f3c5c1bfd4308 (make ip_tables reentrant) introduced a race in
handling the stackptr restore, at the end of ipt_do_table()

We should do it before the call to xt_info_rdunlock_bh(), or we allow
cpu preemption and another cpu overwrites stackptr of original one.

A second fix is to change the underflow test to check the origptr value
instead of 0 to detect underflow, or else we allow a jump from different
hooks.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Jan Engelhardt <jengelh@medozas.de>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 net/ipv4/netfilter/ip_tables.c  |    4 ++--
 net/ipv6/netfilter/ip6_tables.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index b09ed0d..ffcea0d 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -387,7 +387,7 @@ ipt_do_table(struct sk_buff *skb,
 					verdict = (unsigned)(-v) - 1;
 					break;
 				}
-				if (*stackptr == 0) {
+				if (*stackptr <= origptr) {
 					e = get_entry(table_base,
 					    private->underflow[hook]);
 					pr_debug("Underflow (this is normal) "
@@ -427,10 +427,10 @@ ipt_do_table(struct sk_buff *skb,
 			/* Verdict */
 			break;
 	} while (!acpar.hotdrop);
-	xt_info_rdunlock_bh();
 	pr_debug("Exiting %s; resetting sp from %u to %u\n",
 		 __func__, *stackptr, origptr);
 	*stackptr = origptr;
+	xt_info_rdunlock_bh();
 #ifdef DEBUG_ALLOW_ALL
 	return NF_ACCEPT;
 #else
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index c9598a9..0b2af9b 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -410,7 +410,7 @@ ip6t_do_table(struct sk_buff *skb,
 					verdict = (unsigned)(-v) - 1;
 					break;
 				}
-				if (*stackptr == 0)
+				if (*stackptr <= origptr)
 					e = get_entry(table_base,
 					    private->underflow[hook]);
 				else
@@ -441,8 +441,8 @@ ip6t_do_table(struct sk_buff *skb,
 			break;
 	} while (!acpar.hotdrop);
 
-	xt_info_rdunlock_bh();
 	*stackptr = origptr;
+	xt_info_rdunlock_bh();
 
 #ifdef DEBUG_ALLOW_ALL
 	return NF_ACCEPT;
-- 
1.7.4


^ permalink raw reply related

* [PATCH 2/4] netfilter: ipset: fix checking the type revision at create command
From: kaber @ 2011-03-20 15:32 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev
In-Reply-To: <1300635148-22648-1-git-send-email-kaber@trash.net>

From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

The revision of the set type was not checked at the create command: if the
userspace sent a valid set type but with not supported revision number,
it'd create a loop.

Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 net/netfilter/ipset/ip_set_core.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 618a615..d6b4823 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -94,16 +94,28 @@ static int
 find_set_type_get(const char *name, u8 family, u8 revision,
 		  struct ip_set_type **found)
 {
+	struct ip_set_type *type;
+	int err;
+
 	rcu_read_lock();
 	*found = find_set_type(name, family, revision);
 	if (*found) {
-		int err = !try_module_get((*found)->me);
-		rcu_read_unlock();
-		return err ? -EFAULT : 0;
+		err = !try_module_get((*found)->me) ? -EFAULT : 0;
+		goto unlock;
 	}
+	/* Make sure the type is loaded but we don't support the revision */
+	list_for_each_entry_rcu(type, &ip_set_type_list, list)
+		if (STREQ(type->name, name)) {
+			err = -IPSET_ERR_FIND_TYPE;
+			goto unlock;
+		}
 	rcu_read_unlock();
 
 	return try_to_load_type(name);
+
+unlock:
+	rcu_read_unlock();
+	return err;
 }
 
 /* Find a given set type by name and family.
@@ -116,7 +128,7 @@ find_set_type_minmax(const char *name, u8 family, u8 *min, u8 *max)
 	struct ip_set_type *type;
 	bool found = false;
 
-	*min = *max = 0;
+	*min = 255; *max = 0;
 	rcu_read_lock();
 	list_for_each_entry_rcu(type, &ip_set_type_list, list)
 		if (STREQ(type->name, name) &&
@@ -124,7 +136,7 @@ find_set_type_minmax(const char *name, u8 family, u8 *min, u8 *max)
 			found = true;
 			if (type->revision < *min)
 				*min = type->revision;
-			else if (type->revision > *max)
+			if (type->revision > *max)
 				*max = type->revision;
 		}
 	rcu_read_unlock();
-- 
1.7.4


^ permalink raw reply related

* [PATCH 1/4] netfilter: ipset: fix address ranges at hash:*port* types
From: kaber @ 2011-03-20 15:32 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev
In-Reply-To: <1300635148-22648-1-git-send-email-kaber@trash.net>

From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

The hash:*port* types with IPv4 silently ignored when address ranges
with non TCP/UDP were added/deleted from the set and used the first
address from the range only.

Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 include/linux/netfilter/ipset/ip_set_getport.h |   10 +++++++
 net/netfilter/ipset/ip_set_hash_ipport.c       |   34 +++++++-----------------
 net/netfilter/ipset/ip_set_hash_ipportip.c     |   34 +++++++-----------------
 net/netfilter/ipset/ip_set_hash_ipportnet.c    |   34 +++++++-----------------
 net/netfilter/ipset/ip_set_hash_netport.c      |   30 +++++---------------
 5 files changed, 48 insertions(+), 94 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set_getport.h b/include/linux/netfilter/ipset/ip_set_getport.h
index 3882a81..5aebd17 100644
--- a/include/linux/netfilter/ipset/ip_set_getport.h
+++ b/include/linux/netfilter/ipset/ip_set_getport.h
@@ -18,4 +18,14 @@ static inline bool ip_set_get_ip6_port(const struct sk_buff *skb, bool src,
 extern bool ip_set_get_ip_port(const struct sk_buff *skb, u8 pf, bool src,
 				__be16 *port);
 
+static inline bool ip_set_proto_with_ports(u8 proto)
+{
+	switch (proto) {
+	case IPPROTO_TCP:
+	case IPPROTO_UDP:
+		return true;
+	}
+	return false;
+}
+
 #endif /*_IP_SET_GETPORT_H*/
diff --git a/net/netfilter/ipset/ip_set_hash_ipport.c b/net/netfilter/ipset/ip_set_hash_ipport.c
index adbe787..b921414 100644
--- a/net/netfilter/ipset/ip_set_hash_ipport.c
+++ b/net/netfilter/ipset/ip_set_hash_ipport.c
@@ -150,6 +150,7 @@ hash_ipport4_uadt(struct ip_set *set, struct nlattr *tb[],
 	struct hash_ipport4_elem data = { };
 	u32 ip, ip_to, p, port, port_to;
 	u32 timeout = h->timeout;
+	bool with_ports = false;
 	int ret;
 
 	if (unlikely(!tb[IPSET_ATTR_IP] ||
@@ -172,21 +173,15 @@ hash_ipport4_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	if (tb[IPSET_ATTR_PROTO]) {
 		data.proto = nla_get_u8(tb[IPSET_ATTR_PROTO]);
+		with_ports = ip_set_proto_with_ports(data.proto);
 
 		if (data.proto == 0)
 			return -IPSET_ERR_INVALID_PROTO;
 	} else
 		return -IPSET_ERR_MISSING_PROTO;
 
-	switch (data.proto) {
-	case IPPROTO_UDP:
-	case IPPROTO_TCP:
-	case IPPROTO_ICMP:
-		break;
-	default:
+	if (!(with_ports || data.proto == IPPROTO_ICMP))
 		data.port = 0;
-		break;
-	}
 
 	if (tb[IPSET_ATTR_TIMEOUT]) {
 		if (!with_timeout(h->timeout))
@@ -195,7 +190,6 @@ hash_ipport4_uadt(struct ip_set *set, struct nlattr *tb[],
 	}
 
 	if (adt == IPSET_TEST ||
-	    !(data.proto == IPPROTO_TCP || data.proto == IPPROTO_UDP) ||
 	    !(tb[IPSET_ATTR_IP_TO] || tb[IPSET_ATTR_CIDR] ||
 	      tb[IPSET_ATTR_PORT_TO])) {
 		ret = adtfn(set, &data, timeout);
@@ -219,13 +213,12 @@ hash_ipport4_uadt(struct ip_set *set, struct nlattr *tb[],
 	} else
 		ip_to = ip;
 
-	port = ntohs(data.port);
-	if (tb[IPSET_ATTR_PORT_TO]) {
+	port_to = port = ntohs(data.port);
+	if (with_ports && tb[IPSET_ATTR_PORT_TO]) {
 		port_to = ip_set_get_h16(tb[IPSET_ATTR_PORT_TO]);
 		if (port > port_to)
 			swap(port, port_to);
-	} else
-		port_to = port;
+	}
 
 	for (; !before(ip_to, ip); ip++)
 		for (p = port; p <= port_to; p++) {
@@ -361,6 +354,7 @@ hash_ipport6_uadt(struct ip_set *set, struct nlattr *tb[],
 	struct hash_ipport6_elem data = { };
 	u32 port, port_to;
 	u32 timeout = h->timeout;
+	bool with_ports = false;
 	int ret;
 
 	if (unlikely(!tb[IPSET_ATTR_IP] ||
@@ -385,21 +379,15 @@ hash_ipport6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	if (tb[IPSET_ATTR_PROTO]) {
 		data.proto = nla_get_u8(tb[IPSET_ATTR_PROTO]);
+		with_ports = ip_set_proto_with_ports(data.proto);
 
 		if (data.proto == 0)
 			return -IPSET_ERR_INVALID_PROTO;
 	} else
 		return -IPSET_ERR_MISSING_PROTO;
 
-	switch (data.proto) {
-	case IPPROTO_UDP:
-	case IPPROTO_TCP:
-	case IPPROTO_ICMPV6:
-		break;
-	default:
+	if (!(with_ports || data.proto == IPPROTO_ICMPV6))
 		data.port = 0;
-		break;
-	}
 
 	if (tb[IPSET_ATTR_TIMEOUT]) {
 		if (!with_timeout(h->timeout))
@@ -407,9 +395,7 @@ hash_ipport6_uadt(struct ip_set *set, struct nlattr *tb[],
 		timeout = ip_set_timeout_uget(tb[IPSET_ATTR_TIMEOUT]);
 	}
 
-	if (adt == IPSET_TEST ||
-	    !(data.proto == IPPROTO_TCP || data.proto == IPPROTO_UDP) ||
-	    !tb[IPSET_ATTR_PORT_TO]) {
+	if (adt == IPSET_TEST || !with_ports || !tb[IPSET_ATTR_PORT_TO]) {
 		ret = adtfn(set, &data, timeout);
 		return ip_set_eexist(ret, flags) ? 0 : ret;
 	}
diff --git a/net/netfilter/ipset/ip_set_hash_ipportip.c b/net/netfilter/ipset/ip_set_hash_ipportip.c
index 22e23ab..4642872 100644
--- a/net/netfilter/ipset/ip_set_hash_ipportip.c
+++ b/net/netfilter/ipset/ip_set_hash_ipportip.c
@@ -154,6 +154,7 @@ hash_ipportip4_uadt(struct ip_set *set, struct nlattr *tb[],
 	struct hash_ipportip4_elem data = { };
 	u32 ip, ip_to, p, port, port_to;
 	u32 timeout = h->timeout;
+	bool with_ports = false;
 	int ret;
 
 	if (unlikely(!tb[IPSET_ATTR_IP] || !tb[IPSET_ATTR_IP2] ||
@@ -180,21 +181,15 @@ hash_ipportip4_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	if (tb[IPSET_ATTR_PROTO]) {
 		data.proto = nla_get_u8(tb[IPSET_ATTR_PROTO]);
+		with_ports = ip_set_proto_with_ports(data.proto);
 
 		if (data.proto == 0)
 			return -IPSET_ERR_INVALID_PROTO;
 	} else
 		return -IPSET_ERR_MISSING_PROTO;
 
-	switch (data.proto) {
-	case IPPROTO_UDP:
-	case IPPROTO_TCP:
-	case IPPROTO_ICMP:
-		break;
-	default:
+	if (!(with_ports || data.proto == IPPROTO_ICMP))
 		data.port = 0;
-		break;
-	}
 
 	if (tb[IPSET_ATTR_TIMEOUT]) {
 		if (!with_timeout(h->timeout))
@@ -203,7 +198,6 @@ hash_ipportip4_uadt(struct ip_set *set, struct nlattr *tb[],
 	}
 
 	if (adt == IPSET_TEST ||
-	    !(data.proto == IPPROTO_TCP || data.proto == IPPROTO_UDP) ||
 	    !(tb[IPSET_ATTR_IP_TO] || tb[IPSET_ATTR_CIDR] ||
 	      tb[IPSET_ATTR_PORT_TO])) {
 		ret = adtfn(set, &data, timeout);
@@ -227,13 +221,12 @@ hash_ipportip4_uadt(struct ip_set *set, struct nlattr *tb[],
 	} else
 		ip_to = ip;
 
-	port = ntohs(data.port);
-	if (tb[IPSET_ATTR_PORT_TO]) {
+	port_to = port = ntohs(data.port);
+	if (with_ports && tb[IPSET_ATTR_PORT_TO]) {
 		port_to = ip_set_get_h16(tb[IPSET_ATTR_PORT_TO]);
 		if (port > port_to)
 			swap(port, port_to);
-	} else
-		port_to = port;
+	}
 
 	for (; !before(ip_to, ip); ip++)
 		for (p = port; p <= port_to; p++) {
@@ -375,6 +368,7 @@ hash_ipportip6_uadt(struct ip_set *set, struct nlattr *tb[],
 	struct hash_ipportip6_elem data = { };
 	u32 port, port_to;
 	u32 timeout = h->timeout;
+	bool with_ports = false;
 	int ret;
 
 	if (unlikely(!tb[IPSET_ATTR_IP] || !tb[IPSET_ATTR_IP2] ||
@@ -403,21 +397,15 @@ hash_ipportip6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	if (tb[IPSET_ATTR_PROTO]) {
 		data.proto = nla_get_u8(tb[IPSET_ATTR_PROTO]);
+		with_ports = ip_set_proto_with_ports(data.proto);
 
 		if (data.proto == 0)
 			return -IPSET_ERR_INVALID_PROTO;
 	} else
 		return -IPSET_ERR_MISSING_PROTO;
 
-	switch (data.proto) {
-	case IPPROTO_UDP:
-	case IPPROTO_TCP:
-	case IPPROTO_ICMPV6:
-		break;
-	default:
+	if (!(with_ports || data.proto == IPPROTO_ICMPV6))
 		data.port = 0;
-		break;
-	}
 
 	if (tb[IPSET_ATTR_TIMEOUT]) {
 		if (!with_timeout(h->timeout))
@@ -425,9 +413,7 @@ hash_ipportip6_uadt(struct ip_set *set, struct nlattr *tb[],
 		timeout = ip_set_timeout_uget(tb[IPSET_ATTR_TIMEOUT]);
 	}
 
-	if (adt == IPSET_TEST ||
-	    !(data.proto == IPPROTO_TCP || data.proto == IPPROTO_UDP) ||
-	    !tb[IPSET_ATTR_PORT_TO]) {
+	if (adt == IPSET_TEST || !with_ports || !tb[IPSET_ATTR_PORT_TO]) {
 		ret = adtfn(set, &data, timeout);
 		return ip_set_eexist(ret, flags) ? 0 : ret;
 	}
diff --git a/net/netfilter/ipset/ip_set_hash_ipportnet.c b/net/netfilter/ipset/ip_set_hash_ipportnet.c
index 6033e8b..2cb84a5 100644
--- a/net/netfilter/ipset/ip_set_hash_ipportnet.c
+++ b/net/netfilter/ipset/ip_set_hash_ipportnet.c
@@ -174,6 +174,7 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 	struct hash_ipportnet4_elem data = { .cidr = HOST_MASK };
 	u32 ip, ip_to, p, port, port_to;
 	u32 timeout = h->timeout;
+	bool with_ports = false;
 	int ret;
 
 	if (unlikely(!tb[IPSET_ATTR_IP] || !tb[IPSET_ATTR_IP2] ||
@@ -208,21 +209,15 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	if (tb[IPSET_ATTR_PROTO]) {
 		data.proto = nla_get_u8(tb[IPSET_ATTR_PROTO]);
+		with_ports = ip_set_proto_with_ports(data.proto);
 
 		if (data.proto == 0)
 			return -IPSET_ERR_INVALID_PROTO;
 	} else
 		return -IPSET_ERR_MISSING_PROTO;
 
-	switch (data.proto) {
-	case IPPROTO_UDP:
-	case IPPROTO_TCP:
-	case IPPROTO_ICMP:
-		break;
-	default:
+	if (!(with_ports || data.proto == IPPROTO_ICMP))
 		data.port = 0;
-		break;
-	}
 
 	if (tb[IPSET_ATTR_TIMEOUT]) {
 		if (!with_timeout(h->timeout))
@@ -231,7 +226,6 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 	}
 
 	if (adt == IPSET_TEST ||
-	    !(data.proto == IPPROTO_TCP || data.proto == IPPROTO_UDP) ||
 	    !(tb[IPSET_ATTR_IP_TO] || tb[IPSET_ATTR_CIDR] ||
 	      tb[IPSET_ATTR_PORT_TO])) {
 		ret = adtfn(set, &data, timeout);
@@ -255,13 +249,12 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 	} else
 		ip_to = ip;
 
-	port = ntohs(data.port);
-	if (tb[IPSET_ATTR_PORT_TO]) {
+	port_to = port = ntohs(data.port);
+	if (with_ports && tb[IPSET_ATTR_PORT_TO]) {
 		port_to = ip_set_get_h16(tb[IPSET_ATTR_PORT_TO]);
 		if (port > port_to)
 			swap(port, port_to);
-	} else
-		port_to = port;
+	}
 
 	for (; !before(ip_to, ip); ip++)
 		for (p = port; p <= port_to; p++) {
@@ -429,6 +422,7 @@ hash_ipportnet6_uadt(struct ip_set *set, struct nlattr *tb[],
 	struct hash_ipportnet6_elem data = { .cidr = HOST_MASK };
 	u32 port, port_to;
 	u32 timeout = h->timeout;
+	bool with_ports = false;
 	int ret;
 
 	if (unlikely(!tb[IPSET_ATTR_IP] || !tb[IPSET_ATTR_IP2] ||
@@ -465,21 +459,15 @@ hash_ipportnet6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	if (tb[IPSET_ATTR_PROTO]) {
 		data.proto = nla_get_u8(tb[IPSET_ATTR_PROTO]);
+		with_ports = ip_set_proto_with_ports(data.proto);
 
 		if (data.proto == 0)
 			return -IPSET_ERR_INVALID_PROTO;
 	} else
 		return -IPSET_ERR_MISSING_PROTO;
 
-	switch (data.proto) {
-	case IPPROTO_UDP:
-	case IPPROTO_TCP:
-	case IPPROTO_ICMPV6:
-		break;
-	default:
+	if (!(with_ports || data.proto == IPPROTO_ICMPV6))
 		data.port = 0;
-		break;
-	}
 
 	if (tb[IPSET_ATTR_TIMEOUT]) {
 		if (!with_timeout(h->timeout))
@@ -487,9 +475,7 @@ hash_ipportnet6_uadt(struct ip_set *set, struct nlattr *tb[],
 		timeout = ip_set_timeout_uget(tb[IPSET_ATTR_TIMEOUT]);
 	}
 
-	if (adt == IPSET_TEST ||
-	    !(data.proto == IPPROTO_TCP || data.proto == IPPROTO_UDP) ||
-	    !tb[IPSET_ATTR_PORT_TO]) {
+	if (adt == IPSET_TEST || !with_ports || !tb[IPSET_ATTR_PORT_TO]) {
 		ret = adtfn(set, &data, timeout);
 		return ip_set_eexist(ret, flags) ? 0 : ret;
 	}
diff --git a/net/netfilter/ipset/ip_set_hash_netport.c b/net/netfilter/ipset/ip_set_hash_netport.c
index 34a1656..8598676 100644
--- a/net/netfilter/ipset/ip_set_hash_netport.c
+++ b/net/netfilter/ipset/ip_set_hash_netport.c
@@ -170,6 +170,7 @@ hash_netport4_uadt(struct ip_set *set, struct nlattr *tb[],
 	struct hash_netport4_elem data = { .cidr = HOST_MASK };
 	u32 port, port_to;
 	u32 timeout = h->timeout;
+	bool with_ports = false;
 	int ret;
 
 	if (unlikely(!tb[IPSET_ATTR_IP] ||
@@ -198,21 +199,15 @@ hash_netport4_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	if (tb[IPSET_ATTR_PROTO]) {
 		data.proto = nla_get_u8(tb[IPSET_ATTR_PROTO]);
+		with_ports = ip_set_proto_with_ports(data.proto);
 
 		if (data.proto == 0)
 			return -IPSET_ERR_INVALID_PROTO;
 	} else
 		return -IPSET_ERR_MISSING_PROTO;
 
-	switch (data.proto) {
-	case IPPROTO_UDP:
-	case IPPROTO_TCP:
-	case IPPROTO_ICMP:
-		break;
-	default:
+	if (!(with_ports || data.proto == IPPROTO_ICMP))
 		data.port = 0;
-		break;
-	}
 
 	if (tb[IPSET_ATTR_TIMEOUT]) {
 		if (!with_timeout(h->timeout))
@@ -220,9 +215,7 @@ hash_netport4_uadt(struct ip_set *set, struct nlattr *tb[],
 		timeout = ip_set_timeout_uget(tb[IPSET_ATTR_TIMEOUT]);
 	}
 
-	if (adt == IPSET_TEST ||
-	    !(data.proto == IPPROTO_TCP || data.proto == IPPROTO_UDP) ||
-	    !tb[IPSET_ATTR_PORT_TO]) {
+	if (adt == IPSET_TEST || !with_ports || !tb[IPSET_ATTR_PORT_TO]) {
 		ret = adtfn(set, &data, timeout);
 		return ip_set_eexist(ret, flags) ? 0 : ret;
 	}
@@ -390,6 +383,7 @@ hash_netport6_uadt(struct ip_set *set, struct nlattr *tb[],
 	struct hash_netport6_elem data = { .cidr = HOST_MASK };
 	u32 port, port_to;
 	u32 timeout = h->timeout;
+	bool with_ports = false;
 	int ret;
 
 	if (unlikely(!tb[IPSET_ATTR_IP] ||
@@ -418,21 +412,15 @@ hash_netport6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	if (tb[IPSET_ATTR_PROTO]) {
 		data.proto = nla_get_u8(tb[IPSET_ATTR_PROTO]);
+		with_ports = ip_set_proto_with_ports(data.proto);
 
 		if (data.proto == 0)
 			return -IPSET_ERR_INVALID_PROTO;
 	} else
 		return -IPSET_ERR_MISSING_PROTO;
 
-	switch (data.proto) {
-	case IPPROTO_UDP:
-	case IPPROTO_TCP:
-	case IPPROTO_ICMPV6:
-		break;
-	default:
+	if (!(with_ports || data.proto == IPPROTO_ICMPV6))
 		data.port = 0;
-		break;
-	}
 
 	if (tb[IPSET_ATTR_TIMEOUT]) {
 		if (!with_timeout(h->timeout))
@@ -440,9 +428,7 @@ hash_netport6_uadt(struct ip_set *set, struct nlattr *tb[],
 		timeout = ip_set_timeout_uget(tb[IPSET_ATTR_TIMEOUT]);
 	}
 
-	if (adt == IPSET_TEST ||
-	    !(data.proto == IPPROTO_TCP || data.proto == IPPROTO_UDP) ||
-	    !tb[IPSET_ATTR_PORT_TO]) {
+	if (adt == IPSET_TEST || !with_ports || !tb[IPSET_ATTR_PORT_TO]) {
 		ret = adtfn(set, &data, timeout);
 		return ip_set_eexist(ret, flags) ? 0 : ret;
 	}
-- 
1.7.4


^ permalink raw reply related

* Re: [PATCH v2] ipv4: netfilter: ipt_CLUSTERIP: fix buffer overflow
From: Patrick McHardy @ 2011-03-20 14:43 UTC (permalink / raw)
  To: Changli Gao
  Cc: Vasiliy Kulikov, linux-kernel, security, David S. Miller,
	Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	netfilter-devel, netfilter, coreteam, netdev
In-Reply-To: <AANLkTimtsB0B98E2xcoJgRsPwSpwuG+ahT+LX0s1YwpX@mail.gmail.com>

Am 17.03.2011 13:15, schrieb Changli Gao:
> On Thu, Mar 17, 2011 at 7:32 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
>> 'buffer' string is copied from userspace.  It is not checked whether it is
>> zero terminated.  This may lead to overflow inside of simple_strtoul().
>> Changli Gao suggested to copy not more than user supplied 'size' bytes.
>>
>> It was introduced before the git epoch.  Files "ipt_CLUSTERIP/*" are
>> root writable only by default, however, on some setups permissions might be
>> relaxed to e.g. network admin user.
>>
>> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> Acked-by: Changli Gao <xiaosuo@gmail.com>
> 
> 

Applied, thanks everyone.

^ permalink raw reply

* Re: [PATCH 2/2] netfilter: get rid of atomic ops in fast path
From: Patrick McHardy @ 2011-03-20 14:41 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Netfilter Development Mailinglist, netdev,
	Jan Engelhardt
In-Reply-To: <1300468038.2888.160.camel@edumazet-laptop>

Am 18.03.2011 18:07, schrieb Eric Dumazet:
> We currently use a percpu spinlock to 'protect' rule bytes/packets
> counters, after various attempts to use RCU instead.
> 
> Lately we added a seqlock so that get_counters() can run without
> blocking BH or 'writers'. But we really use the seqcount in it.
> 
> Spinlock itself is only locked by the current/owner cpu, so we can
> remove it completely.
> 
> This cleanups api, using correct 'writer' vs 'reader' semantic.
> 
> At replace time, the get_counters() call makes sure all cpus are done
> using the old table.

I think this will have to wait until net-next opens up again since
its not a bugfix.

^ permalink raw reply

* Re: [PATCH 1/2] netfilter: xtables: fix reentrancy
From: Patrick McHardy @ 2011-03-20 14:40 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Netfilter Development Mailinglist, netdev,
	Jan Engelhardt
In-Reply-To: <1300467921.2888.157.camel@edumazet-laptop>

Am 18.03.2011 18:05, schrieb Eric Dumazet:
> commit f3c5c1bfd4308 (make ip_tables reentrant) introduced a race in
> handling the stackptr restore, at the end of ipt_do_table()
> 
> We should do it before the call to xt_info_rdunlock_bh(), or we allow
> cpu preemption and another cpu overwrites stackptr of original one.
> 
> A second fix is to change the underflow test to check the origptr value
> instead of 0 to detect underflow, or else we allow a jump from different
> hooks.

Applied, thanks Eric.

^ permalink raw reply

* Re: [PATCH] socket: increase default maximum listen queue length
From: Hagen Paul Pfeifer @ 2011-03-20 12:14 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1300622144.2831.287.camel@edumazet-laptop>

* Eric Dumazet | 2011-03-20 12:55:44 [+0100]:

>I am not sure you understood what I said.
>
>Even if you change kernel limits, many applications still use low
>limits : listen(fd, 8)

Right, but there is a discrepance between system administrators and server
authors: the later group will probably notice that listen(fd, 8) is not
adequate (e.g. someone send a bug report). System administrators on the other
hand have no obvious indicator that some goes wrong in the system. Most of
then would not even notice that the backlog is overflowing. 

>I remember some other OS (was it HPUX or Solaris...) had a minimum
>limit : Even if application said 8, an admin could impose a 256 value
>for example.

Not the baddest idea! It is nice that a server author can adjust that value.
But between you and me: the system administrator may have more information
about the network behavior (how many incoming connections/minute, RTT, memory
characteristic, ...). The system administrator should be in the ability to
increase the value, currently he is stucked up if the server author missed
that. E.g.

http://www.dovecot.org/list/dovecot-cvs/2009-September/014567.html

I will spin a patch for that.

Hagen

^ permalink raw reply

* Re: [PATCH] socket: increase default maximum listen queue length
From: Hagen Paul Pfeifer @ 2011-03-20 11:59 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, eric.dumazet
In-Reply-To: <20110319.214100.183043711.davem@davemloft.net>

* David Miller | 2011-03-19 21:41:00 [-0700]:

>What in the world is a server running on multi-GHZ cpus doing such
>that it cannot accept() fast enough to keep up with a 100 connections
>per second?
>
>We were handling that just fine 10+ years ago.
>
>The math simply doesn't add up.

Davem, what the hell - you don't get it! ;-) No Davem, the problem is not the
accept() performance rather it is the time since SYN/ACK is arrived and
request_sock are created: RTT time to the client (including client SYN/ACK
processing) and finally the accept() processing time. During _this_ duration
the backlog is of relevance. For connection with a high RTT the backlog can be
quite high - local accept() processing delay does not really matter.

The actual behavior is not really fine. max_syn_backlog and somaxconn are
somewhat[TM] unsynchronized. max_syn_backlog considers the system memory
characteristic where somaxconn is just a dump show stopper. My patch is a
compromise it increase the value to 256 because it showed up that 256 seems
reasonable for many setups. Many system administrators will not notice the
problem, server authors may know about this limitation (and adjust the listen
argument)- so why should we make the life of the administrators a little bit
more easy by adjusting the default? Low memory systems does not suffer, the
limit is still reasonable small. The memory footprint for a server with 4
listening sockets is just insignificant. But connection failure due to backlog
limits will magically go away.

The math is a function from incoming connections / time, the RTT and
processing delay of client and server side..

Have a nice Sunday, Hagen

^ permalink raw reply

* Re: [PATCH] socket: increase default maximum listen queue length
From: Eric Dumazet @ 2011-03-20 11:55 UTC (permalink / raw)
  To: Hagen Paul Pfeifer; +Cc: netdev
In-Reply-To: <20110320113921.GA3038@nuttenaction>

Le dimanche 20 mars 2011 à 12:39 +0100, Hagen Paul Pfeifer a écrit :
> * Eric Dumazet | 2011-03-20 09:30:17 [+0100]:
> 
> >Hmm, real problem is not the 'maximum queue value', but the minimum one.
> >
> >If application says : listen(fd, 10), you are stuck.
> >
> >128 or 256 is way too small on some servers, where admin can tune
> >in /etc/sysctl.conf :
> >
> >net.core.somaxconn = 8192
> >net.ipv4.tcp_max_syn_backlog = 8192
> >
> >But application also needs to use : listen(fd, 8192)
> 
> I know Eric, I wrote the patch. ;-) The used naming goes like this:
> 
> The system limits (somaxconn & tcp_max_syn_backlog) specify a _maximum_, the
> user cannot exceed this limit with listen(2). The backlog argument for listen
> on the other hand specify a _minimum_. But the patch increased the default
> maximum, therefore I named it in this way.


I am not sure you understood what I said.

Even if you change kernel limits, many applications still use low
limits : listen(fd, 8)

I remember some other OS (was it HPUX or Solaris...) had a minimum
limit : Even if application said 8, an admin could impose a 256 value
for example.

Frankly, I believe somaxconn should be a default enforcement, that a
particular protocol could override.

TCP sockets for example should only enforce tcp_max_syn_backlog limit,
not the somaxconn & tcp_max_syn_backlog.




^ permalink raw reply

* Re: [PATCH] socket: increase default maximum listen queue length
From: Hagen Paul Pfeifer @ 2011-03-20 11:39 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1300609817.2831.56.camel@edumazet-laptop>

* Eric Dumazet | 2011-03-20 09:30:17 [+0100]:

>Hmm, real problem is not the 'maximum queue value', but the minimum one.
>
>If application says : listen(fd, 10), you are stuck.
>
>128 or 256 is way too small on some servers, where admin can tune
>in /etc/sysctl.conf :
>
>net.core.somaxconn = 8192
>net.ipv4.tcp_max_syn_backlog = 8192
>
>But application also needs to use : listen(fd, 8192)

I know Eric, I wrote the patch. ;-) The used naming goes like this:

The system limits (somaxconn & tcp_max_syn_backlog) specify a _maximum_, the
user cannot exceed this limit with listen(2). The backlog argument for listen
on the other hand specify a _minimum_. But the patch increased the default
maximum, therefore I named it in this way.

^ permalink raw reply

* Re: [PATCH V3] Export ACPI _DSM provided firmware instance number and string name to sysfs
From: Alexander Beregalov @ 2011-03-20 10:15 UTC (permalink / raw)
  To: Narendra_K
  Cc: mjg59, linux-pci, linux-hotplug, netdev, Matt_Domsch,
	Charles_Rose, Jordan_Hargrave, Shyam_Iyer
In-Reply-To: <20110223132147.GB16707@fedora14-r610.blr.amer.dell.com>

On 23 February 2011 16:00,  <Narendra_K@dell.com> wrote:
> On Wed, Feb 23, 2011 at 06:14:19PM +0530, Matthew Garrett wrote:
>> I think this version will still break the build. You need to depend on
>> CONFIG_NLS.
>
> Matthew,
>
> Thanks. I posted a patch to linux-next to fix the build failure.

Hi,
it still does not work

drivers/pci/Kconfig:
select NLS if (DMI || ACPI)

 $ egrep "G_ACPI=|_DMI=|G_NLS=" .config
CONFIG_DMI=y
CONFIG_ACPI=y
CONFIG_NLS=m

pci-label.c:(.text+0xb80a): undefined reference to `utf16s_to_utf8s'

^ permalink raw reply

* Re: [PATCH] socket: increase default maximum listen queue length
From: Eric Dumazet @ 2011-03-20  9:36 UTC (permalink / raw)
  To: Rémi Denis-Courmont; +Cc: Hagen Paul Pfeifer, netdev
In-Reply-To: <201103201104.42183.remi@remlab.net>

Le dimanche 20 mars 2011 à 11:04 +0200, Rémi Denis-Courmont a écrit :
> Le dimanche 20 mars 2011 10:30:17 Eric Dumazet, vous avez écrit :
> > 
> > But application also needs to use : listen(fd, 8192)
> 
> Application should pass INT_MAX anyway, unless it has a specific limit, e.g. 
> it can only handle one connection at time.
> 

You'll be surprised how few of them do that actually

ss -a | head
State      Recv-Q Send-Q      Local Address:Port          Peer Address:Port   
LISTEN     0      8                       *:imaps                    *:*       
LISTEN     0      8                       *:pop3s                    *:*       
LISTEN     0      50                      *:mysql                    *:*       
LISTEN     0      8                       *:pop3                     *:*       
LISTEN     0      8                       *:imap2                    *:*       
LISTEN     0      511                     *:www                      *:*       


Yes, Apache itself uses 511 as its default listenbacklog

You'll need to change 

ListenBacklog 8192




^ permalink raw reply

* Re: [PATCH] Add useful per-connection TCP stats for diagnosis purpose.
From: Eric Dumazet @ 2011-03-20  9:18 UTC (permalink / raw)
  To: Jerry Chu; +Cc: Stephen Hemminger, netdev
In-Reply-To: <AANLkTikyoS-YDH+N92yCXiFg7MfXhSHeudND9VJgAp15@mail.gmail.com>

Le dimanche 20 mars 2011 à 00:30 -0700, Jerry Chu a écrit :

> I presume the filtering of specific IP addrs/ports are done in the
> kernel to short-circuit the
> O(n) loop. Otherwise it seems moot (i.e., it still costs O(n)).
> 

Filtering is done in kernel, and we can extend filtering capa if
necessary.

/proc/net/tcp is a flat file, you cannot do this


> A quick look at inet_csk_diag_dump() does not reveal anything like
> that. But I haven't
> looked hard enough.

You should ;)



^ permalink raw reply

* Re: [PATCH] socket: increase default maximum listen queue length
From: Rémi Denis-Courmont @ 2011-03-20  9:04 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Hagen Paul Pfeifer, netdev
In-Reply-To: <1300609817.2831.56.camel@edumazet-laptop>

Le dimanche 20 mars 2011 10:30:17 Eric Dumazet, vous avez écrit :
> 
> But application also needs to use : listen(fd, 8192)

Application should pass INT_MAX anyway, unless it has a specific limit, e.g. 
it can only handle one connection at time.

-- 
Rémi Denis-Courmont
http://www.remlab.info/
http://fi.linkedin.com/in/remidenis

^ permalink raw reply

* Re: [PATCH] socket: increase default maximum listen queue length
From: Eric Dumazet @ 2011-03-20  8:30 UTC (permalink / raw)
  To: Hagen Paul Pfeifer; +Cc: netdev
In-Reply-To: <1300585811-21566-1-git-send-email-hagen@jauu.net>

Le dimanche 20 mars 2011 à 02:50 +0100, Hagen Paul Pfeifer a écrit :
> sysctl_somaxconn (SOMAXCONN: 128) specifies the maximum number of
> sockets in state SYN_RECV per listen socket queue. At listen(2) time the
> backlog is adjusted to this limit if bigger then that.
> 
> Afterwards in reqsk_queue_alloc() the backlog value is checked again
> (nr_table_entries == backlog):
> 
>     nr_table_entries = min_t(u32, nr_table_entries, sysctl_max_syn_backlog);
>     nr_table_entries = max_t(u32, nr_table_entries, 8);
>     nr_table_entries = roundup_pow_of_two(nr_table_entries + 1);
> 
> sysctl_max_syn_backlog on the other hand is dynamically adjusted,
> depending on the memory characteristic of the system. Default is 256,
> 128 for small systems and up to 1024 for bigger systems.
> 
> For real server work the defacto sysctl_somaxconn limit seems inadequate:
> 
>     Experiments with real servers show, that it is absolutely not enough
>     even at 100conn/sec. 256 cures most of problems.
> 
> Increase default sysctl_somaxconn from 128 to 256 to meet todays condition by
> simultaneously limit nr_table_entries by sysctl_max_syn_backlog which is
> based on memory condition (max(128, (tcp_hashinfo.ehash_mask + 1 / 256)).
> 
> Signed_off-by: Hagen Paul Pfeifer <hagen@jauu.net>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  include/linux/socket.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index edbb1d0..bf35ce2 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -237,7 +237,7 @@ struct ucred {
>  #define PF_MAX		AF_MAX
>  
>  /* Maximum queue length specifiable by listen.  */
> -#define SOMAXCONN	128
> +#define SOMAXCONN	256
>  
>  /* Flags we can use with send/ and recv. 
>     Added those for 1003.1g not all are supported yet


Hmm, real problem is not the 'maximum queue value', but the minimum one.

If application says : listen(fd, 10), you are stuck.

128 or 256 is way too small on some servers, where admin can tune
in /etc/sysctl.conf :

net.core.somaxconn = 8192
net.ipv4.tcp_max_syn_backlog = 8192

But application also needs to use : listen(fd, 8192)




^ permalink raw reply

* [PATCH 1/1] solos-pci: Fix debugging to include size, vpi/vci, but omit preamble
From: Philip Prindeville @ 2011-03-20  7:47 UTC (permalink / raw)
  To: Netdev; +Cc: linux-atm-general@lists.sourceforge.net

Show the buffer length, vpi, and vci as we do on receive.  Use the word "port" instead of "device" consistently.

Don't display the "struct pkt_hdr" preamble on the sk_buff when doing hexdump of frame data.

Signed-off-by: Philip Prindeville <philipp@redfish-solutions.com>
---

--- a/drivers/atm/solos-pci.c.orig	2011-03-19 12:19:06.000000000 -0600
+++ b/drivers/atm/solos-pci.c	2011-03-19 12:45:18.000000000 -0600
@@ -697,7 +697,7 @@ void solos_bh(unsigned long card_arg)
  					      size);
  			}
  			if (atmdebug) {
-				dev_info(&card->dev->dev, "Received: device %d\n", port);
+				dev_info(&card->dev->dev, "Received: port %d\n", port);
  				dev_info(&card->dev->dev, "size: %d VPI: %d VCI: %d\n",
  					 size, le16_to_cpu(header->vpi),
  					 le16_to_cpu(header->vci));
@@ -1025,8 +1025,15 @@ static uint32_t fpga_tx(struct solos_car

  			/* Clean up and free oldskb now it's gone */
  			if (atmdebug) {
+				struct pkt_hdr *header = (void *)oldskb->data;
+				int size = le16_to_cpu(header->size);
+
+				skb_pull(oldskb, sizeof(*header));
  				dev_info(&card->dev->dev, "Transmitted: port %d\n",
  					 port);
+				dev_info(&card->dev->dev, "size: %d VPI: %d VCI: %d\n",
+					 size, le16_to_cpu(header->vpi),
+					 le16_to_cpu(header->vci));
  				print_buffer(oldskb);
  			}




^ permalink raw reply

* Re: [PATCH] Add useful per-connection TCP stats for diagnosis purpose.
From: Jerry Chu @ 2011-03-20  7:30 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Stephen Hemminger, netdev
In-Reply-To: <1300514604.2831.23.camel@edumazet-laptop>

On Fri, Mar 18, 2011 at 11:03 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> Le vendredi 18 mars 2011 à 22:38 -0700, Jerry Chu a écrit :
> > On Fri, Mar 18, 2011 at 10:02 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > Le vendredi 18 mars 2011 à 21:33 -0700, Jerry Chu a écrit :
> > >
> > >> I'm well aware of the past, hideous O(n**2) problem of reading
> > >> /proc/net/tcp but I
> > >> thought the problem has been fixed by Tom Herbert a while back, no?
> > >> (See http://marc.info/?l=linux-netdev&m=127588123429437&w=2)
> > >>
> > >
> > > O(N) is still too slow, to gather stats for your socket, if you want to
> > > gather stats once per second for example.
> >
> > I don't see anyway around O(n). The netlink/inet_diag_dump() will take at
> > best O(n) too.
>
> 1) netlink is extensible, without breaking old apps.
>
> 2) netlink is able to dump part of the information, while
> whith /proc/net/tcp you have to cat all the file and grep the needed
> info. O(1) is better than O(10)
>
> Example :
>
> I only want info of socket identified by dst ip 192.168.0.144 and dst
> port 58196
>
> # ss -emoi dst 192.168.20.144:58196
> State       Recv-Q Send-Q     Local Address:Port         Peer Address:Port
> ESTAB       0      0         192.168.20.108:65111      192.168.20.144:58196    ino:3302 sk:ffff88011c278c80
>         mem:(r0,w0,f0,t0) sack cubic wscale:7,7 rto:201 rtt:1.75/0.75 cwnd:10 send 66.7Mbps rcv_space:14600
>
> I want all sockets to remote 192.168.20.110
> # ss -emoi dst 192.168.20.110
> State       Recv-Q Send-Q     Local Address:Port         Peer Address:Port
> ESTAB       0      0      ::ffff:192.168.20.108:ssh     ::ffff:192.168.20.110:47633    timer:(keepalive,120min,0) ino:70342 sk:ffff88011c381500
>         mem:(r0,w0,f0,t0) ts sack ecn cubic wscale:8,7 rto:207 rtt:7.625/11.5 ato:40 cwnd:10 send 15.2Mbps rcv_rtt:1 rcv_space:14480

I presume the filtering of specific IP addrs/ports are done in the
kernel to short-circuit the
O(n) loop. Otherwise it seems moot (i.e., it still costs O(n)).

A quick look at inet_csk_diag_dump() does not reveal anything like
that. But I haven't
looked hard enough.

>
>
> 3) getsockopt(fd) is O(1)
>
> >
> > >
> > > AFAIK, I am not sure we want to allow any user to access all these data
> > > for all tcp sockets on the machine. This might have security impacts.
> >
> > This seems to be an orthogonal issue to netlink or /proc/net/tcp.
>
> getsockopt(fd) only finds information about your own socket.

Make no mistake if my usage case was driven by specific apps why would I
waste my time chasing /proc/net/tcp? In fact we've had a private
TCP_INFO_EXT socket option for years used by some specific apps. (We
have not tried to pushed it upstreams because some of the counters we collect
are too Google specific.)

But my immediate use of the counters/stats proposed here is for our internal
monitor programs to take samples occasionally from ALL CONNECTIONS, or for
diagnosis purpose from the TCP layer when we don't have specific info about the
apps' IP or port# (so we just want to dump info from all connections).

>
> An application might want to log tcp information only right before
> closing one socket. This sounds more practical than having a separate
> spy thread.

I'm not trying to use /proc/net/tcp (or netlink) because I don't know about
other, more efficient way to retrieve counters from a subset, or even just one
specific connection as explained above.

Jerry

>
>
>

^ permalink raw reply

* [PATCH v2] ROSE: prevent heap corruption with bad facilities
From: Dan Rosenberg @ 2011-03-20  6:43 UTC (permalink / raw)
  To: ralf, davem; +Cc: netdev, security

When parsing the FAC_NATIONAL_DIGIS facilities field, it's possible for
a remote host to provide more digipeaters than expected, resulting in
heap corruption.  Check against ROSE_MAX_DIGIS to prevent overflows, and
abort facilities parsing on failure.

Additionally, when parsing the FAC_CCITT_DEST_NSAP and
FAC_CCITT_SRC_NSAP facilities fields, a remote host can provide a length
of less than 10, resulting in an underflow in a memcpy size, causing a
kernel panic due to massive heap corruption.  A length of greater than
20 results in a stack overflow of the callsign array.  Abort facilities
parsing on these invalid length values.

Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
Cc: stable@kernel.org
---
 net/rose/rose_subr.c |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/net/rose/rose_subr.c b/net/rose/rose_subr.c
index 1734abb..6f5f0b5 100644
--- a/net/rose/rose_subr.c
+++ b/net/rose/rose_subr.c
@@ -290,10 +290,15 @@ static int rose_parse_national(unsigned char *p, struct rose_facilities_struct *
 				facilities->source_ndigis = 0;
 				facilities->dest_ndigis   = 0;
 				for (pt = p + 2, lg = 0 ; lg < l ; pt += AX25_ADDR_LEN, lg += AX25_ADDR_LEN) {
-					if (pt[6] & AX25_HBIT)
+					if (pt[6] & AX25_HBIT) {
+						if (facilities->dest_ndigis >= ROSE_MAX_DIGIS)
+							return -1;
 						memcpy(&facilities->dest_digis[facilities->dest_ndigis++], pt, AX25_ADDR_LEN);
-					else
+					} else {
+						if (facilities->source_ndigis >= ROSE_MAX_DIGIS)
+							return -1;
 						memcpy(&facilities->source_digis[facilities->source_ndigis++], pt, AX25_ADDR_LEN);
+					}
 				}
 			}
 			p   += l + 2;
@@ -333,6 +338,11 @@ static int rose_parse_ccitt(unsigned char *p, struct rose_facilities_struct *fac
 
 		case 0xC0:
 			l = p[1];
+
+			/* Prevent overflows*/
+			if (l < 10 || l > 20)
+				return -1;
+
 			if (*p == FAC_CCITT_DEST_NSAP) {
 				memcpy(&facilities->source_addr, p + 7, ROSE_ADDR_LEN);
 				memcpy(callsign, p + 12,   l - 10);
@@ -373,12 +383,16 @@ int rose_parse_facilities(unsigned char *p,
 			switch (*p) {
 			case FAC_NATIONAL:		/* National */
 				len = rose_parse_national(p + 1, facilities, facilities_len - 1);
+				if (len < 0)
+					return 0;
 				facilities_len -= len + 1;
 				p += len + 1;
 				break;
 
 			case FAC_CCITT:		/* CCITT */
 				len = rose_parse_ccitt(p + 1, facilities, facilities_len - 1);
+				if (len < 0)
+					return 0;
 				facilities_len -= len + 1;
 				p += len + 1;
 				break;



^ permalink raw reply related

* Re: [PATCH] niu: Rename NIU parent platform device name to fix conflict.
From: Grant Likely @ 2011-03-20  6:27 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20110319.230733.193709340.davem@davemloft.net>

On Sun, Mar 20, 2011 at 12:07 AM, David Miller <davem@davemloft.net> wrote:
>
> When the OF device driver bits were converted over to the platform
> device infrastructure in commit 74888760d40b3ac9054f9c5fa07b566c0676ba2d
> ("dt/net: Eliminate users of of_platform_{,un}register_driver") we
> inadvertantly created probing problems in the OF case.
[...]
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>
> Grant, I'll push this fix via the net-2.6 tree, you don't have
> to worry about it.

Okay, thanks.

g.

>
>  drivers/net/niu.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/niu.c b/drivers/net/niu.c
> index 40fa59e..32678b6 100644
> --- a/drivers/net/niu.c
> +++ b/drivers/net/niu.c
> @@ -9501,7 +9501,7 @@ static struct niu_parent * __devinit niu_new_parent(struct niu *np,
>        struct niu_parent *p;
>        int i;
>
> -       plat_dev = platform_device_register_simple("niu", niu_parent_index,
> +       plat_dev = platform_device_register_simple("niu-board", niu_parent_index,
>                                                   NULL, 0);
>        if (IS_ERR(plat_dev))
>                return NULL;
> --
> 1.7.4.1
>
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* [PATCH v2] irda: prevent heap corruption on invalid nickname
From: Dan Rosenberg @ 2011-03-20  6:14 UTC (permalink / raw)
  To: samuel, davem; +Cc: netdev, security

Invalid nicknames containing only spaces will result in an underflow in
a memcpy size calculation, subsequently destroying the heap and
panicking.

v2 also catches the case where the provided nickname is longer than the
buffer size, which can result in controllable heap corruption.

Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
Cc: stable@kernel.org
---
 net/irda/irnet/irnet_ppp.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/irda/irnet/irnet_ppp.c b/net/irda/irnet/irnet_ppp.c
index 7c567b8..2bb2beb 100644
--- a/net/irda/irnet/irnet_ppp.c
+++ b/net/irda/irnet/irnet_ppp.c
@@ -105,6 +105,9 @@ irnet_ctrl_write(irnet_socket *	ap,
 	      while(isspace(start[length - 1]))
 		length--;
 
+	      DABORT(length < 5 || length > NICKNAME_MAX_LEN + 5,
+		     -EINVAL, CTRL_ERROR, "Invalid nickname.\n");
+
 	      /* Copy the name for later reuse */
 	      memcpy(ap->rname, start + 5, length - 5);
 	      ap->rname[length - 5] = '\0';



^ permalink raw reply related

* Re: [PATCH] ROSE: prevent heap corruption with bad facilities
From: David Miller @ 2011-03-20  6:07 UTC (permalink / raw)
  To: drosenberg; +Cc: ralf, netdev, security
In-Reply-To: <1300601004.1869.1.camel@dan>

From: Dan Rosenberg <drosenberg@vsecurity.com>
Date: Sun, 20 Mar 2011 02:03:24 -0400

> Please disregard this patch, my brain wasn't working properly when I
> wrote it.  There are problems in these areas, but this fix is incomplete
> (and the description for the first issue is practically nonsensical).
> I'll resend a new version shortly.

Ok.

^ permalink raw reply

* [PATCH] niu: Rename NIU parent platform device name to fix conflict.
From: David Miller @ 2011-03-20  6:07 UTC (permalink / raw)
  To: grant.likely; +Cc: netdev


When the OF device driver bits were converted over to the platform
device infrastructure in commit 74888760d40b3ac9054f9c5fa07b566c0676ba2d
("dt/net: Eliminate users of of_platform_{,un}register_driver") we
inadvertantly created probing problems in the OF case.

The NIU driver creates a dummy platform device to represent the
board that contains one or more child NIU devices.  Unfortunately
we use the same name, "niu", as the OF device driver itself uses.

The result is that we try to probe the dummy "niu" parent device we
create, and since it has a NULL ofdevice pointer etc. everything
explodes:

[783019.128243] niu: niu.c:v1.1 (Apr 22, 2010)
[783019.128810] Unable to handle kernel NULL pointer dereference
[783019.128949] tsk->{mm,active_mm}->context = 000000000000039e
[783019.129078] tsk->{mm,active_mm}->pgd = fffff803afc5a000
[783019.129206]               \|/ ____ \|/
[783019.129213]               "@'/ .. \`@"
[783019.129220]               /_| \__/ |_\
[783019.129226]                  \__U_/
[783019.129378] modprobe(2004): Oops [#1]
[783019.129423] TSTATE: 0000000011001602 TPC: 0000000010052ff8 TNPC: 000000000061bbb4 Y: 00000000    Not tainted
[783019.129542] TPC: <niu_of_probe+0x3c/0x2dc [niu]>
[783019.129624] g0: 8080000000000000 g1: 0000000000000000 g2: 0000000010056000 g3: 0000000000000002
[783019.129733] g4: fffff803fc1da0c0 g5: fffff800441e2000 g6: fffff803fba84000 g7: 0000000000000000
[783019.129842] o0: fffff803fe7df010 o1: 0000000010055700 o2: 0000000000000000 o3: fffff803fbacaca0
[783019.129951] o4: 0000000000000080 o5: 0000000000777908 sp: fffff803fba866e1 ret_pc: 0000000010052ff4
[783019.130083] RPC: <niu_of_probe+0x38/0x2dc [niu]>
[783019.130165] l0: fffff803fe7df010 l1: fffff803fbacafc0 l2: fffff803fbacaca0 l3: ffffffffffffffed
[783019.130273] l4: 0000000000000000 l5: 000000007fffffff l6: fffff803fba86f40 l7: 0000000000000001
[783019.130382] i0: fffff803fe7df000 i1: fffff803fc20aba0 i2: 0000000000000000 i3: 0000000000000001
[783019.130490] i4: 0000000000000000 i5: 0000000000000000 i6: fffff803fba867a1 i7: 000000000062038c
[783019.130614] I7: <platform_drv_probe+0xc/0x20>

Fix by simply renaming the parent device to "niu-board".

Signed-off-by: David S. Miller <davem@davemloft.net>
---

Grant, I'll push this fix via the net-2.6 tree, you don't have
to worry about it.

 drivers/net/niu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index 40fa59e..32678b6 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -9501,7 +9501,7 @@ static struct niu_parent * __devinit niu_new_parent(struct niu *np,
 	struct niu_parent *p;
 	int i;
 
-	plat_dev = platform_device_register_simple("niu", niu_parent_index,
+	plat_dev = platform_device_register_simple("niu-board", niu_parent_index,
 						   NULL, 0);
 	if (IS_ERR(plat_dev))
 		return NULL;
-- 
1.7.4.1


^ permalink raw reply related

* Re: [PATCH] ROSE: prevent heap corruption with bad facilities
From: Dan Rosenberg @ 2011-03-20  6:03 UTC (permalink / raw)
  To: ralf; +Cc: davem, netdev, security
In-Reply-To: <1300577313.12507.17.camel@dan>

On Sat, 2011-03-19 at 19:28 -0400, Dan Rosenberg wrote:
> When parsing the FAC_NATIONAL_DIGIS facilities field, keeping the
> counter field in an unsigned char is insufficient, since the counter is
> incremented by AX25_ADDR_LEN (7) with each loop.  Providing a field
> length of 0xf, for example, causes heap corruption because the counter
> wraps without ever reaching the length and data is copied past the
> boundaries of the source_digis or dest_digis facilities arrays.  Change
> the counter to an unsigned ints to prevent this wrapping and overflow.
> 
> Additionally, when parsing the FAC_CCITT_DEST_NSAP and
> FAC_CCITT_SRC_NSAP facilities fields, a length of less than 10 results
> in an underflow in a memcpy size, resulting in a kernel panic due to
> massive heap corruption.  Abort facilities parsing on this invalid
> length value.

Please disregard this patch, my brain wasn't working properly when I
wrote it.  There are problems in these areas, but this fix is incomplete
(and the description for the first issue is practically nonsensical).
I'll resend a new version shortly.

> 
> Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
> Cc: stable@kernel.org
> ---
>  net/rose/rose_subr.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/net/rose/rose_subr.c b/net/rose/rose_subr.c
> index 1734abb..fee9de4 100644
> --- a/net/rose/rose_subr.c
> +++ b/net/rose/rose_subr.c
> @@ -240,7 +240,8 @@ int rose_decode(struct sk_buff *skb, int *ns, int *nr, int *q, int *d, int *m)
>  static int rose_parse_national(unsigned char *p, struct rose_facilities_struct *facilities, int len)
>  {
>  	unsigned char *pt;
> -	unsigned char l, lg, n = 0;
> +	unsigned char l, n = 0;
> +	unsigned int lg;
>  	int fac_national_digis_received = 0;
>  
>  	do {
> @@ -334,12 +335,16 @@ static int rose_parse_ccitt(unsigned char *p, struct rose_facilities_struct *fac
>  		case 0xC0:
>  			l = p[1];
>  			if (*p == FAC_CCITT_DEST_NSAP) {
> +				if (l < 10)
> +					return -1;
>  				memcpy(&facilities->source_addr, p + 7, ROSE_ADDR_LEN);
>  				memcpy(callsign, p + 12,   l - 10);
>  				callsign[l - 10] = '\0';
>  				asc2ax(&facilities->source_call, callsign);
>  			}
>  			if (*p == FAC_CCITT_SRC_NSAP) {
> +				if (l < 10)
> +					return -1;
>  				memcpy(&facilities->dest_addr, p + 7, ROSE_ADDR_LEN);
>  				memcpy(callsign, p + 12, l - 10);
>  				callsign[l - 10] = '\0';
> @@ -379,6 +384,11 @@ int rose_parse_facilities(unsigned char *p,
>  
>  			case FAC_CCITT:		/* CCITT */
>  				len = rose_parse_ccitt(p + 1, facilities, facilities_len - 1);
> +
> +				/* Invalid facilities */
> +				if (len < 0)
> +					return 0;
> +
>  				facilities_len -= len + 1;
>  				p += len + 1;
>  				break;
> 



^ permalink raw reply

* Re: [PATCH] socket: increase default maximum listen queue length
From: David Miller @ 2011-03-20  4:41 UTC (permalink / raw)
  To: hagen; +Cc: netdev, eric.dumazet
In-Reply-To: <1300585811-21566-1-git-send-email-hagen@jauu.net>

From: Hagen Paul Pfeifer <hagen@jauu.net>
Date: Sun, 20 Mar 2011 02:50:11 +0100

> For real server work the defacto sysctl_somaxconn limit seems inadequate:
> 
>     Experiments with real servers show, that it is absolutely not enough
>     even at 100conn/sec. 256 cures most of problems.

What in the world is a server running on multi-GHZ cpus doing such
that it cannot accept() fast enough to keep up with a 100 connections
per second?

We were handling that just fine 10+ years ago.

The math simply doesn't add up.

Either your numbers are wrong or the server design is brain-dead.

^ permalink raw reply

* [PATCH] socket: increase default maximum listen queue length
From: Hagen Paul Pfeifer @ 2011-03-20  1:50 UTC (permalink / raw)
  To: netdev; +Cc: Hagen Paul Pfeifer, Eric Dumazet

sysctl_somaxconn (SOMAXCONN: 128) specifies the maximum number of
sockets in state SYN_RECV per listen socket queue. At listen(2) time the
backlog is adjusted to this limit if bigger then that.

Afterwards in reqsk_queue_alloc() the backlog value is checked again
(nr_table_entries == backlog):

    nr_table_entries = min_t(u32, nr_table_entries, sysctl_max_syn_backlog);
    nr_table_entries = max_t(u32, nr_table_entries, 8);
    nr_table_entries = roundup_pow_of_two(nr_table_entries + 1);

sysctl_max_syn_backlog on the other hand is dynamically adjusted,
depending on the memory characteristic of the system. Default is 256,
128 for small systems and up to 1024 for bigger systems.

For real server work the defacto sysctl_somaxconn limit seems inadequate:

    Experiments with real servers show, that it is absolutely not enough
    even at 100conn/sec. 256 cures most of problems.

Increase default sysctl_somaxconn from 128 to 256 to meet todays condition by
simultaneously limit nr_table_entries by sysctl_max_syn_backlog which is
based on memory condition (max(128, (tcp_hashinfo.ehash_mask + 1 / 256)).

Signed_off-by: Hagen Paul Pfeifer <hagen@jauu.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/linux/socket.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/socket.h b/include/linux/socket.h
index edbb1d0..bf35ce2 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -237,7 +237,7 @@ struct ucred {
 #define PF_MAX		AF_MAX
 
 /* Maximum queue length specifiable by listen.  */
-#define SOMAXCONN	128
+#define SOMAXCONN	256
 
 /* Flags we can use with send/ and recv. 
    Added those for 1003.1g not all are supported yet
-- 
1.7.4.1.57.g0466.dirty


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox