[PATCH] netfilter: ipset: hash:net,iface: fix interface comparision

[PATCH] netfilter: ipset: hash:net,iface: fix interface comparision

[Florian Westphal]

ifname_compare() assumes that skb->dev is zero-padded,
e.g 'eth1\0\0\0\0\0...'. This isn't always the case. e1000 driver does

strncpy(netdev->name, pci_name(pdev), sizeof(netdev->name) - 1);

in e1000_probe(), so once device is registered dev->name memory contains
'eth1\0:0:3\0\0\0' (or something like that), which makes eth1 compare
fail.

Use plain strcmp() instead.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/ipset/ip_set_hash_netiface.c |   32 +++------------------------
 1 files changed, 4 insertions(+), 28 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
index ee86394..d5d3607 100644
--- a/net/netfilter/ipset/ip_set_hash_netiface.c
+++ b/net/netfilter/ipset/ip_set_hash_netiface.c
@@ -38,30 +38,6 @@ struct iface_node {
 
 #define iface_data(n)	(rb_entry(n, struct iface_node, node)->iface)
 
-static inline long
-ifname_compare(const char *_a, const char *_b)
-{
-	const long *a = (const long *)_a;
-	const long *b = (const long *)_b;
-
-	BUILD_BUG_ON(IFNAMSIZ > 4 * sizeof(unsigned long));
-	if (a[0] != b[0])
-		return a[0] - b[0];
-	if (IFNAMSIZ > sizeof(long)) {
-		if (a[1] != b[1])
-			return a[1] - b[1];
-	}
-	if (IFNAMSIZ > 2 * sizeof(long)) {
-		if (a[2] != b[2])
-			return a[2] - b[2];
-	}
-	if (IFNAMSIZ > 3 * sizeof(long)) {
-		if (a[3] != b[3])
-			return a[3] - b[3];
-	}
-	return 0;
-}
-
 static void
 rbtree_destroy(struct rb_root *root)
 {
@@ -99,7 +75,7 @@ iface_test(struct rb_root *root, const char **iface)
 
 	while (n) {
 		const char *d = iface_data(n);
-		long res = ifname_compare(*iface, d);
+		int res = strcmp(*iface, d);
 
 		if (res < 0)
 			n = n->rb_left;
@@ -121,7 +97,7 @@ iface_add(struct rb_root *root, const char **iface)
 
 	while (*n) {
 		char *ifname = iface_data(*n);
-		long res = ifname_compare(*iface, ifname);
+		int res = strcmp(*iface, ifname);
 
 		p = *n;
 		if (res < 0)
@@ -366,7 +342,7 @@ hash_netiface4_uadt(struct ip_set *set, struct nlattr *tb[],
 	struct hash_netiface4_elem data = { .cidr = HOST_MASK };
 	u32 ip = 0, ip_to, last;
 	u32 timeout = h->timeout;
-	char iface[IFNAMSIZ] = {};
+	char iface[IFNAMSIZ];
 	int ret;
 
 	if (unlikely(!tb[IPSET_ATTR_IP] ||
@@ -663,7 +639,7 @@ hash_netiface6_uadt(struct ip_set *set, struct nlattr *tb[],
 	ipset_adtfn adtfn = set->variant->adt[adt];
 	struct hash_netiface6_elem data = { .cidr = HOST_MASK };
 	u32 timeout = h->timeout;
-	char iface[IFNAMSIZ] = {};
+	char iface[IFNAMSIZ];
 	int ret;
 
 	if (unlikely(!tb[IPSET_ATTR_IP] ||
-- 
1.7.3.4

Re: [PATCH] netfilter: ipset: hash:net,iface: fix interface comparision

[Pablo Neira Ayuso]

On Sun, Jun 17, 2012 at 09:56:46PM +0200, Florian Westphal wrote:
> ifname_compare() assumes that skb->dev is zero-padded,
> e.g 'eth1\0\0\0\0\0...'. This isn't always the case. e1000 driver does
> 
> strncpy(netdev->name, pci_name(pdev), sizeof(netdev->name) - 1);
> 
> in e1000_probe(), so once device is registered dev->name memory contains
> 'eth1\0:0:3\0\0\0' (or something like that), which makes eth1 compare
> fail.
> 
> Use plain strcmp() instead.

Applied, thanks Florian.

Re: [PATCH] netfilter: ipset: hash:net,iface: fix interface comparision

[Jozsef Kadlecsik]

On Mon, 25 Jun 2012, Pablo Neira Ayuso wrote:

> On Sun, Jun 17, 2012 at 09:56:46PM +0200, Florian Westphal wrote:
> > ifname_compare() assumes that skb->dev is zero-padded,
> > e.g 'eth1\0\0\0\0\0...'. This isn't always the case. e1000 driver does
> > 
> > strncpy(netdev->name, pci_name(pdev), sizeof(netdev->name) - 1);
> > 
> > in e1000_probe(), so once device is registered dev->name memory contains
> > 'eth1\0:0:3\0\0\0' (or something like that), which makes eth1 compare
> > fail.
> > 
> > Use plain strcmp() instead.
> 
> Applied, thanks Florian.

Thanks, Pablo! I had commited the patch in my ipset git tree, just haven't 
time yet to send/ack to you.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH] netfilter: ipset: hash:net,iface: fix interface comparision

[Pablo Neira Ayuso]

On Tue, Jun 26, 2012 at 09:08:31AM +0200, Jozsef Kadlecsik wrote:
> On Mon, 25 Jun 2012, Pablo Neira Ayuso wrote:
> 
> > On Sun, Jun 17, 2012 at 09:56:46PM +0200, Florian Westphal wrote:
> > > ifname_compare() assumes that skb->dev is zero-padded,
> > > e.g 'eth1\0\0\0\0\0...'. This isn't always the case. e1000 driver does
> > > 
> > > strncpy(netdev->name, pci_name(pdev), sizeof(netdev->name) - 1);
> > > 
> > > in e1000_probe(), so once device is registered dev->name memory contains
> > > 'eth1\0:0:3\0\0\0' (or something like that), which makes eth1 compare
> > > fail.
> > > 
> > > Use plain strcmp() instead.
> > 
> > Applied, thanks Florian.
> 
> Thanks, Pablo! I had commited the patch in my ipset git tree, just haven't 
> time yet to send/ack to you.

No problem. Please, rebase your tree to remove it:

git rebase -i COMMIT_YOU_WANT_TO_REMOVE^
# then, write "e" in the patch you want to remove on the list that it shows
git reset HEAD^
git reset --hard
git rebase --continue

Not sure if there's a smarter way to do it. This is how I make it
though.

Note that we're on -rc4, so please send me important fixes that you
want me to pass to David first, after that those to net-next.

Thanks Jozsef.