[NETFILTER 00/09]: Netfilter fixes

[NETFILTER 00/09]: Netfilter fixes

[Patrick McHardy]

Hi Dave,

these patches contain some netfilter fixes for 2.6.25, fixing undersized
skb allocation and incorrect EPERM errors in nfnetlink_queue and
nfnetlink_log, an xt_time bug that causes mismatches on Sundays, a
missing newline in a conntrack message and the "horrible hack" in
ct_extend.

Please apply, thanks.


 include/linux/netfilter/nfnetlink_compat.h  |    2 +-
 include/net/netfilter/nf_conntrack_extend.h |    1 -
 net/netfilter/nf_conntrack_expect.c         |    2 +-
 net/netfilter/nf_conntrack_extend.c         |   19 ++-------------
 net/netfilter/nf_queue.c                    |    2 +-
 net/netfilter/nfnetlink_log.c               |   32 ++++++++++++++------------
 net/netfilter/nfnetlink_queue.c             |   17 ++++----------
 net/netfilter/xt_time.c                     |    7 ++++-
 8 files changed, 33 insertions(+), 49 deletions(-)

Alexey Dobriyan (1):
      [NETFILTER]: nf_conntrack: add \n to "expectation table full" message

Andrew Schulman (1):
      [NETFILTER]: xt_time: fix failure to match on Sundays

Eric Leblond (2):
      [NETFILTER]: nfnetlink_queue: fix computation of allocated size for netlink skb.
      [NETFILTER]: nfnetlink_log: fix computation of netlink skb size

Patrick McHardy (4):
      [NETFILTER]: nfnetlink: fix ifdef in nfnetlink_compat.h
      [NETFILTER]; nfnetlink_log: fix EPERM when binding/unbinding and instance 0 exists
      [NETFILTER]: nfnetlink_queue: fix EPERM when binding/unbinding and instance 0 exists
      [NETFILTER]: nf_queue: don't return error when unregistering a non-existant handler

Pekka Enberg (1):
      [NETFILTER]: nf_conntrack: replace horrible hack with ksize()

[NETFILTER 01/09]: nfnetlink: fix ifdef in nfnetlink_compat.h

[Patrick McHardy]

[NETFILTER]: nfnetlink: fix ifdef in nfnetlink_compat.h

Use __KERNEL__ instead of __KERNEL to make sure the headers are not
usable by the kernel.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit a0866e6699bd8bd0f5016eaf8a194c0f4dfd5f7e
tree 6794b755e7e2fbc0e9318fd6086f0065cc2b1577
parent ac840248700f1675ae55fe629dd7bd0a5eb17c4f
author Patrick McHardy <kaber@trash.net> Mon, 10 Mar 2008 17:50:41 +0100
committer Patrick McHardy <kaber@trash.net> Mon, 10 Mar 2008 17:50:41 +0100

 include/linux/netfilter/nfnetlink_compat.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink_compat.h b/include/linux/netfilter/nfnetlink_compat.h
index 02a42d8..e145176 100644
--- a/include/linux/netfilter/nfnetlink_compat.h
+++ b/include/linux/netfilter/nfnetlink_compat.h
@@ -1,6 +1,6 @@
 #ifndef _NFNETLINK_COMPAT_H
 #define _NFNETLINK_COMPAT_H
-#ifndef __KERNEL
+#ifndef __KERNEL__
 /* Old nfnetlink macros for userspace */
 
 /* nfnetlink groups: Up to 32 maximum */

[NETFILTER 02/09]: nfnetlink_queue: fix computation of allocated size for netlink skb.

[Patrick McHardy]

[NETFILTER]: nfnetlink_queue: fix computation of allocated size for netlink skb.

Size of the netlink skb was wrongly computed because the formula was using
NLMSG_ALIGN instead of NLMSG_SPACE. NLMSG_ALIGN does not add the room for
netlink header as NLMSG_SPACE does. This was causing a failure of message
building in some cases.

On my test system, all messages for packets in range [8*k+41, 8*k+48] where k
is an integer were invalid and the corresponding packets were dropped.

Signed-off-by: Eric Leblond <eric@inl.fr>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit fab7718e07093ec0e51b1193560bae72d29b2ef1
tree 03084ce20b7f13470e8eb046326ac9592572e9f4
parent a0866e6699bd8bd0f5016eaf8a194c0f4dfd5f7e
author Eric Leblond <eric@inl.fr> Mon, 10 Mar 2008 17:50:41 +0100
committer Patrick McHardy <kaber@trash.net> Mon, 10 Mar 2008 17:50:41 +0100

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

diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 385dacd..a573baa 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -224,7 +224,7 @@ nfqnl_build_packet_message(struct nfqnl_instance *queue,
 	struct net_device *indev;
 	struct net_device *outdev;
 
-	size =    NLMSG_ALIGN(sizeof(struct nfgenmsg))
+	size =    NLMSG_SPACE(sizeof(struct nfgenmsg))
 		+ nla_total_size(sizeof(struct nfqnl_msg_packet_hdr))
 		+ nla_total_size(sizeof(u_int32_t))	/* ifindex */
 		+ nla_total_size(sizeof(u_int32_t))	/* ifindex */

[NETFILTER 03/09]: nfnetlink_log: fix computation of netlink skb size

[Patrick McHardy]

[NETFILTER]: nfnetlink_log: fix computation of netlink skb size

This patch is similar to nfnetlink_queue fixes. It fixes the computation
of skb size by using NLMSG_SPACE instead of NLMSG_ALIGN.

Signed-off-by: Eric Leblond <eric@inl.fr>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit b306ccc2129dfc062653b3436df714fc758f3c45
tree 98b4d4d9819246591cd11d38e07dbbad182287b0
parent fab7718e07093ec0e51b1193560bae72d29b2ef1
author Eric Leblond <eric@inl.fr> Mon, 10 Mar 2008 17:50:41 +0100
committer Patrick McHardy <kaber@trash.net> Mon, 10 Mar 2008 17:50:41 +0100

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

diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 83832eb..be71091 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -556,7 +556,7 @@ nfulnl_log_packet(unsigned int pf,
 	/* FIXME: do we want to make the size calculation conditional based on
 	 * what is actually present?  way more branches and checks, but more
 	 * memory efficient... */
-	size =    NLMSG_ALIGN(sizeof(struct nfgenmsg))
+	size =    NLMSG_SPACE(sizeof(struct nfgenmsg))
 		+ nla_total_size(sizeof(struct nfulnl_msg_packet_hdr))
 		+ nla_total_size(sizeof(u_int32_t))	/* ifindex */
 		+ nla_total_size(sizeof(u_int32_t))	/* ifindex */

[NETFILTER 04/09]: xt_time: fix failure to match on Sundays

[Patrick McHardy]

[NETFILTER]: xt_time: fix failure to match on Sundays

From: Andrew Schulman <andrex@alumni.utexas.net>

xt_time_match() in net/netfilter/xt_time.c in kernel 2.6.24 never
matches on Sundays. On my host I have a rule like

    iptables -A OUTPUT -m time --weekdays Sun -j REJECT

and it never matches. The problem is in localtime_2(), which uses

    r->weekday = (4 + r->dse) % 7;

to map the epoch day onto a weekday in {0,...,6}. In particular this
gives 0 for Sundays. But 0 has to be wrong; a weekday of 0 can never
match. xt_time_match() has

    if (!(info->weekdays_match & (1 << current_time.weekday)))
        return false;

and when current_time.weekday = 0, the result of the & is always
zero, even when info->weekdays_match = XT_TIME_ALL_WEEKDAYS = 0xFE.

Signed-off-by: Jan Engelhardt <jengelh@computergmbh.de>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 342661f5a04f90772e0d33302634e010ab3649ed
tree 3dc6d36f9d245b9f2990ab503735904c1d8187d1
parent b306ccc2129dfc062653b3436df714fc758f3c45
author Andrew Schulman <andrex@alumni.utexas.net> Mon, 10 Mar 2008 17:50:42 +0100
committer Patrick McHardy <kaber@trash.net> Mon, 10 Mar 2008 17:50:42 +0100

 net/netfilter/xt_time.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/xt_time.c b/net/netfilter/xt_time.c
index e9a8794..9fa2e08 100644
--- a/net/netfilter/xt_time.c
+++ b/net/netfilter/xt_time.c
@@ -95,8 +95,11 @@ static inline void localtime_2(struct xtm *r, time_t time)
 	 */
 	r->dse = time / 86400;
 
-	/* 1970-01-01 (w=0) was a Thursday (4). */
-	r->weekday = (4 + r->dse) % 7;
+	/*
+	 * 1970-01-01 (w=0) was a Thursday (4).
+	 * -1 and +1 map Sunday properly onto 7.
+	 */
+	r->weekday = (4 + r->dse - 1) % 7 + 1;
 }
 
 static void localtime_3(struct xtm *r, time_t time)

[NETFILTER 05/09]: nf_conntrack: add \n to "expectation table full" message

[Patrick McHardy]

[NETFILTER]: nf_conntrack: add \n to "expectation table full" message

Signed-off-by: Alexey Dobriyan <adobriyan@sw.ru>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 6cc9434319dd0ea32cc6e1969523a1a77043379c
tree 54392c76361f72bed435bd740a84ad3d0aa8a07a
parent 342661f5a04f90772e0d33302634e010ab3649ed
author Alexey Dobriyan <adobriyan@sw.ru> Mon, 10 Mar 2008 17:50:42 +0100
committer Patrick McHardy <kaber@trash.net> Mon, 10 Mar 2008 17:50:42 +0100

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

diff --git a/net/netfilter/nf_conntrack_expect.c b/net/netfilter/nf_conntrack_expect.c
index e06bf00..684ec9c 100644
--- a/net/netfilter/nf_conntrack_expect.c
+++ b/net/netfilter/nf_conntrack_expect.c
@@ -381,7 +381,7 @@ int nf_ct_expect_related(struct nf_conntrack_expect *expect)
 	if (nf_ct_expect_count >= nf_ct_expect_max) {
 		if (net_ratelimit())
 			printk(KERN_WARNING
-			       "nf_conntrack: expectation table full");
+			       "nf_conntrack: expectation table full\n");
 		ret = -EMFILE;
 		goto out;
 	}

[NETFILTER 06/09]: nf_conntrack: replace horrible hack with ksize()

[Patrick McHardy]

[NETFILTER]: nf_conntrack: replace horrible hack with ksize()

There's a horrible slab abuse in net/netfilter/nf_conntrack_extend.c that
can be replaced with a call to ksize().

Cc: Christoph Lameter <clameter@sgi.com>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 2bb076fe75d4f6542d7a769c6430a8332b776896
tree b1449912958a87758aceea5fe8581e38425a448d
parent 6cc9434319dd0ea32cc6e1969523a1a77043379c
author Pekka Enberg <penberg@cs.helsinki.fi> Mon, 10 Mar 2008 18:51:26 +0100
committer Patrick McHardy <kaber@trash.net> Mon, 10 Mar 2008 18:51:26 +0100

 include/net/netfilter/nf_conntrack_extend.h |    1 -
 net/netfilter/nf_conntrack_extend.c         |   19 +++----------------
 2 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_extend.h b/include/net/netfilter/nf_conntrack_extend.h
index 49aac63..f736e84 100644
--- a/include/net/netfilter/nf_conntrack_extend.h
+++ b/include/net/netfilter/nf_conntrack_extend.h
@@ -17,7 +17,6 @@ enum nf_ct_ext_id
 struct nf_ct_ext {
 	u8 offset[NF_CT_EXT_NUM];
 	u8 len;
-	u8 real_len;
 	char data[0];
 };
 
diff --git a/net/netfilter/nf_conntrack_extend.c b/net/netfilter/nf_conntrack_extend.c
index 8b9be1e..2bd9963 100644
--- a/net/netfilter/nf_conntrack_extend.c
+++ b/net/netfilter/nf_conntrack_extend.c
@@ -19,14 +19,6 @@
 static struct nf_ct_ext_type *nf_ct_ext_types[NF_CT_EXT_NUM];
 static DEFINE_MUTEX(nf_ct_ext_type_mutex);
 
-/* Horrible trick to figure out smallest amount worth kmallocing. */
-#define CACHE(x) (x) + 0 *
-enum {
-	NF_CT_EXT_MIN_SIZE =
-#include <linux/kmalloc_sizes.h>
-	1 };
-#undef CACHE
-
 void __nf_ct_ext_destroy(struct nf_conn *ct)
 {
 	unsigned int i;
@@ -53,7 +45,7 @@ EXPORT_SYMBOL(__nf_ct_ext_destroy);
 static void *
 nf_ct_ext_create(struct nf_ct_ext **ext, enum nf_ct_ext_id id, gfp_t gfp)
 {
-	unsigned int off, len, real_len;
+	unsigned int off, len;
 	struct nf_ct_ext_type *t;
 
 	rcu_read_lock();
@@ -61,16 +53,14 @@ nf_ct_ext_create(struct nf_ct_ext **ext, enum nf_ct_ext_id id, gfp_t gfp)
 	BUG_ON(t == NULL);
 	off = ALIGN(sizeof(struct nf_ct_ext), t->align);
 	len = off + t->len;
-	real_len = t->alloc_size;
 	rcu_read_unlock();
 
-	*ext = kzalloc(real_len, gfp);
+	*ext = kzalloc(t->alloc_size, gfp);
 	if (!*ext)
 		return NULL;
 
 	(*ext)->offset[id] = off;
 	(*ext)->len = len;
-	(*ext)->real_len = real_len;
 
 	return (void *)(*ext) + off;
 }
@@ -95,7 +85,7 @@ void *__nf_ct_ext_add(struct nf_conn *ct, enum nf_ct_ext_id id, gfp_t gfp)
 	newlen = newoff + t->len;
 	rcu_read_unlock();
 
-	if (newlen >= ct->ext->real_len) {
+	if (newlen >= ksize(ct->ext)) {
 		new = kmalloc(newlen, gfp);
 		if (!new)
 			return NULL;
@@ -114,7 +104,6 @@ void *__nf_ct_ext_add(struct nf_conn *ct, enum nf_ct_ext_id id, gfp_t gfp)
 			rcu_read_unlock();
 		}
 		kfree(ct->ext);
-		new->real_len = newlen;
 		ct->ext = new;
 	}
 
@@ -156,8 +145,6 @@ static void update_alloc_size(struct nf_ct_ext_type *type)
 			t1->alloc_size = ALIGN(t1->alloc_size, t2->align)
 					 + t2->len;
 		}
-		if (t1->alloc_size < NF_CT_EXT_MIN_SIZE)
-			t1->alloc_size = NF_CT_EXT_MIN_SIZE;
 	}
 }
 

[NETFILTER 07/09]: nfnetlink_log: fix EPERM when binding/unbinding and instance 0 exists

[Patrick McHardy]

[NETFILTER]: nfnetlink_log: fix EPERM when binding/unbinding and instance 0 exists

When binding or unbinding to an address family, the res_id is usually set
to zero. When logging instance 0 already exists and is owned by a different
process, this makes nfunl_recv_config return -EPERM without performing
the bind operation.

Since no operation on the foreign logging instance itself was requested,
this is incorrect. Move bind/unbind commands before the queue instance
permissions checks.

Also remove an incorrect comment.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 1db33bfdd9496c38b1f3e41b2e6ff72649d99044
tree 9368563942adff2a0fd84a9fe5463af4d4466eb0
parent 2bb076fe75d4f6542d7a769c6430a8332b776896
author Patrick McHardy <kaber@trash.net> Mon, 10 Mar 2008 19:11:12 +0100
committer Patrick McHardy <kaber@trash.net> Mon, 10 Mar 2008 19:11:12 +0100

 net/netfilter/nfnetlink_log.c |   30 ++++++++++++++++--------------
 1 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index be71091..0edacff 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -702,20 +702,30 @@ nfulnl_recv_config(struct sock *ctnl, struct sk_buff *skb,
 	struct nfgenmsg *nfmsg = NLMSG_DATA(nlh);
 	u_int16_t group_num = ntohs(nfmsg->res_id);
 	struct nfulnl_instance *inst;
+	struct nfulnl_msg_config_cmd *cmd = NULL;
 	int ret = 0;
 
+	if (nfula[NFULA_CFG_CMD]) {
+		u_int8_t pf = nfmsg->nfgen_family;
+		cmd = nla_data(nfula[NFULA_CFG_CMD]);
+
+		/* Commands without queue context */
+		switch (cmd->command) {
+		case NFULNL_CFG_CMD_PF_BIND:
+			return nf_log_register(pf, &nfulnl_logger);
+		case NFULNL_CFG_CMD_PF_UNBIND:
+			nf_log_unregister_pf(pf);
+			return 0;
+		}
+	}
+
 	inst = instance_lookup_get(group_num);
 	if (inst && inst->peer_pid != NETLINK_CB(skb).pid) {
 		ret = -EPERM;
 		goto out_put;
 	}
 
-	if (nfula[NFULA_CFG_CMD]) {
-		u_int8_t pf = nfmsg->nfgen_family;
-		struct nfulnl_msg_config_cmd *cmd;
-
-		cmd = nla_data(nfula[NFULA_CFG_CMD]);
-
+	if (cmd != NULL) {
 		switch (cmd->command) {
 		case NFULNL_CFG_CMD_BIND:
 			if (inst) {
@@ -738,14 +748,6 @@ nfulnl_recv_config(struct sock *ctnl, struct sk_buff *skb,
 
 			instance_destroy(inst);
 			goto out;
-		case NFULNL_CFG_CMD_PF_BIND:
-			ret = nf_log_register(pf, &nfulnl_logger);
-			break;
-		case NFULNL_CFG_CMD_PF_UNBIND:
-			/* This is a bug and a feature.  We cannot unregister
-			 * other handlers, like nfnetlink_inst can */
-			nf_log_unregister_pf(pf);
-			break;
 		default:
 			ret = -ENOTSUPP;
 			break;

[NETFILTER 08/09]: nfnetlink_queue: fix EPERM when binding/unbinding and instance 0 exists

[Patrick McHardy]

[NETFILTER]: nfnetlink_queue: fix EPERM when binding/unbinding and instance 0 exists

Similar to the nfnetlink_log problem, nfnetlink_queue incorrectly returns -EPERM
when binding or unbinding to an address family and queueing instance 0 exists
and is owned by a different process. Unlike nfnetlink_log it previously completes
the operation, but it is still incorrect.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 68b176bf4421f595f94c8a80fffcf4cc06d7edc4
tree 3bcd124e75e83a8f7fd8df822bd66fb2d305f812
parent 1db33bfdd9496c38b1f3e41b2e6ff72649d99044
author Patrick McHardy <kaber@trash.net> Mon, 10 Mar 2008 19:11:31 +0100
committer Patrick McHardy <kaber@trash.net> Mon, 10 Mar 2008 19:11:31 +0100

 net/netfilter/nfnetlink_queue.c |   15 ++++-----------
 1 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index a573baa..7f9d820 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -703,19 +703,12 @@ nfqnl_recv_config(struct sock *ctnl, struct sk_buff *skb,
 		/* Commands without queue context - might sleep */
 		switch (cmd->command) {
 		case NFQNL_CFG_CMD_PF_BIND:
-			ret = nf_register_queue_handler(ntohs(cmd->pf),
-							&nfqh);
-			break;
+			return nf_register_queue_handler(ntohs(cmd->pf),
+							 &nfqh);
 		case NFQNL_CFG_CMD_PF_UNBIND:
-			ret = nf_unregister_queue_handler(ntohs(cmd->pf),
-							  &nfqh);
-			break;
-		default:
-			break;
+			return nf_unregister_queue_handler(ntohs(cmd->pf),
+							   &nfqh);
 		}
-
-		if (ret < 0)
-			return ret;
 	}
 
 	rcu_read_lock();

[NETFILTER 09/09]: nf_queue: don't return error when unregistering a non-existant handler

[Patrick McHardy]

[NETFILTER]: nf_queue: don't return error when unregistering a non-existant handler

Commit ce7663d84:

[NETFILTER]: nfnetlink_queue: don't unregister handler of other subsystem

changed nf_unregister_queue_handler to return an error when attempting to
unregister a queue handler that is not identical to the one passed in.
This is correct in case we really do have a different queue handler already
registered, but some existing userspace code always does an unbind before
bind and aborts if that fails, so try to be nice and return success in
that case.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit c5fa0de9a02e442b8dc72db52f5fa9154b73b49d
tree 414b455f6fad5a9839b7e852d9e62d4dc6f7990d
parent 68b176bf4421f595f94c8a80fffcf4cc06d7edc4
author Patrick McHardy <kaber@trash.net> Mon, 10 Mar 2008 19:13:13 +0100
committer Patrick McHardy <kaber@trash.net> Mon, 10 Mar 2008 19:13:13 +0100

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

diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
index 6bfdb78..61b55a5 100644
--- a/net/netfilter/nf_queue.c
+++ b/net/netfilter/nf_queue.c
@@ -51,7 +51,7 @@ int nf_unregister_queue_handler(int pf, const struct nf_queue_handler *qh)
 		return -EINVAL;
 
 	mutex_lock(&queue_handler_mutex);
-	if (queue_handler[pf] != qh) {
+	if (queue_handler[pf] && queue_handler[pf] != qh) {
 		mutex_unlock(&queue_handler_mutex);
 		return -EINVAL;
 	}

Re: [NETFILTER 00/09]: Netfilter fixes

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Mon, 10 Mar 2008 19:26:13 +0100 (MET)

> these patches contain some netfilter fixes for 2.6.25, fixing undersized
> skb allocation and incorrect EPERM errors in nfnetlink_queue and
> nfnetlink_log, an xt_time bug that causes mismatches on Sundays, a
> missing newline in a conntrack message and the "horrible hack" in
> ct_extend.

All applied and pushed out to net-2.6, thanks Patrick!