[PATCH RESEND 0/3] netfilter: xtables: inclusion of condition match and target

[PATCH RESEND 0/3] netfilter: xtables: inclusion of condition match and target

[Luciano Coelho]

Hi,

I'm resending these three patches now that the net-next is open for development
again.  These patches include the xt_condition match and target.

Please integrate if everything is okay.

Cheers,
Luca.

Luciano Coelho (3):
  netfilter: xtables: inclusion of xt_condition
  netfilter: xt_condition: change the value from boolean to u32
  netfilter: xt_condition: add condition target support

 include/linux/netfilter/Kbuild         |    1 +
 include/linux/netfilter/xt_condition.h |   26 +++
 net/netfilter/Kconfig                  |   11 +
 net/netfilter/Makefile                 |    1 +
 net/netfilter/xt_condition.c           |  357 ++++++++++++++++++++++++++++++++
 5 files changed, 396 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/netfilter/xt_condition.h
 create mode 100644 net/netfilter/xt_condition.c

[PATCH RESEND 1/3] netfilter: xtables: inclusion of xt_condition

[Luciano Coelho]

xt_condition can be used by userspace to influence decisions in rules
by means of togglable variables without having to reload the entire
ruleset.

This is a respin of the module in Xtables-addons, with support for
multiple namespaces and other small improvements.  Some of the changes
were made by Jan Engelhardt.

Cc: Jan Engelhardt <jengelh@medozas.de>
Signed-off-by: Luciano Coelho <luciano.coelho@nokia.com>
---
 include/linux/netfilter/Kbuild         |    1 +
 include/linux/netfilter/xt_condition.h |   14 ++
 net/netfilter/Kconfig                  |    8 +
 net/netfilter/Makefile                 |    1 +
 net/netfilter/xt_condition.c           |  265 ++++++++++++++++++++++++++++++++
 5 files changed, 289 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/netfilter/xt_condition.h
 create mode 100644 net/netfilter/xt_condition.c

diff --git a/include/linux/netfilter/Kbuild b/include/linux/netfilter/Kbuild
index 9d40eff..ef8425e 100644
--- a/include/linux/netfilter/Kbuild
+++ b/include/linux/netfilter/Kbuild
@@ -28,6 +28,7 @@ header-y += xt_TEE.h
 header-y += xt_TPROXY.h
 header-y += xt_cluster.h
 header-y += xt_comment.h
+header-y += xt_condition.h
 header-y += xt_connbytes.h
 header-y += xt_connlimit.h
 header-y += xt_connmark.h
diff --git a/include/linux/netfilter/xt_condition.h b/include/linux/netfilter/xt_condition.h
new file mode 100644
index 0000000..4faf3ca
--- /dev/null
+++ b/include/linux/netfilter/xt_condition.h
@@ -0,0 +1,14 @@
+#ifndef _XT_CONDITION_H
+#define _XT_CONDITION_H
+
+#include <linux/types.h>
+
+struct xt_condition_mtinfo {
+	char name[31];
+	__u8 invert;
+
+	/* Used internally by the kernel */
+	void *condvar __attribute__((aligned(8)));
+};
+
+#endif /* _XT_CONDITION_H */
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 4328825..5044dd6 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -621,6 +621,14 @@ config NETFILTER_XT_MATCH_COMMENT
 	  If you want to compile it as a module, say M here and read
 	  <file:Documentation/kbuild/modules.txt>.  If unsure, say `N'.
 
+config NETFILTER_XT_MATCH_CONDITION
+	tristate '"condition" match support'
+	depends on NETFILTER_ADVANCED
+	depends on PROC_FS
+	---help---
+	This option allows you to match firewall rules against condition
+	variables stored in the /proc/net/nf_condition directory.
+
 config NETFILTER_XT_MATCH_CONNBYTES
 	tristate  '"connbytes" per-connection counter match support'
 	depends on NF_CONNTRACK
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 441050f..bbf72bb 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_NETFILTER_XT_TARGET_IDLETIMER) += xt_IDLETIMER.o
 # matches
 obj-$(CONFIG_NETFILTER_XT_MATCH_CLUSTER) += xt_cluster.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_COMMENT) += xt_comment.o
+obj-$(CONFIG_NETFILTER_XT_MATCH_CONDITION) += xt_condition.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_CONNBYTES) += xt_connbytes.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_CONNLIMIT) += xt_connlimit.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_CONNTRACK) += xt_conntrack.o
diff --git a/net/netfilter/xt_condition.c b/net/netfilter/xt_condition.c
new file mode 100644
index 0000000..a78d832
--- /dev/null
+++ b/net/netfilter/xt_condition.c
@@ -0,0 +1,265 @@
+/*
+ *	"condition" match extension for Xtables
+ *
+ *	Description: This module allows firewall rules to match using
+ *	condition variables available through procfs.
+ *
+ *	Authors:
+ *	Stephane Ouellette <ouellettes [at] videotron ca>, 2002-10-22
+ *	Massimiliano Hofer <max [at] nucleus it>, 2006-05-15
+ *
+ *	This program is free software; you can redistribute it and/or modify it
+ *	under the terms of the GNU General Public License; either version 2
+ *	or 3 of the License, as published by the Free Software Foundation.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/proc_fs.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
+#include <linux/version.h>
+#include <linux/netfilter/x_tables.h>
+#include <linux/netfilter/xt_condition.h>
+#include <net/netns/generic.h>
+#include <asm/uaccess.h>
+
+/* Defaults, these can be overridden on the module command-line. */
+static unsigned int condition_list_perms = S_IRUGO | S_IWUSR;
+static unsigned int condition_uid_perms = 0;
+static unsigned int condition_gid_perms = 0;
+
+MODULE_AUTHOR("Stephane Ouellette <ouellettes@videotron.ca>");
+MODULE_AUTHOR("Massimiliano Hofer <max@nucleus.it>");
+MODULE_AUTHOR("Jan Engelhardt <jengelh@medozas.de>");
+MODULE_DESCRIPTION("Allows rules to match against condition variables");
+MODULE_LICENSE("GPL");
+module_param(condition_list_perms, uint, S_IRUSR | S_IWUSR);
+MODULE_PARM_DESC(condition_list_perms, "default permissions on /proc/net/nf_condition/* files");
+module_param(condition_uid_perms, uint, S_IRUSR | S_IWUSR);
+MODULE_PARM_DESC(condition_uid_perms, "default user owner of /proc/net/nf_condition/* files");
+module_param(condition_gid_perms, uint, S_IRUSR | S_IWUSR);
+MODULE_PARM_DESC(condition_gid_perms, "default group owner of /proc/net/nf_condition/* files");
+MODULE_ALIAS("ipt_condition");
+MODULE_ALIAS("ip6t_condition");
+
+struct condition_variable {
+	struct list_head list;
+	struct proc_dir_entry *status_proc;
+	unsigned int refcount;
+	bool enabled;
+};
+
+struct condition_net {
+	struct list_head list;
+	struct proc_dir_entry *proc_dir;
+};
+
+static int condition_net_id;
+static inline struct condition_net *condition_pernet(struct net *net)
+{
+	return net_generic(net, condition_net_id);
+}
+
+/* proc_lock is a user context only semaphore used for write access */
+/*           to the conditions' list.                               */
+static DEFINE_MUTEX(proc_lock);
+
+static int condition_proc_read(char __user *buffer, char **start, off_t offset,
+			       int length, int *eof, void *data)
+{
+	const struct condition_variable *var = data;
+
+	buffer[0] = var->enabled ? '1' : '0';
+	buffer[1] = '\n';
+	if (length >= 2)
+		*eof = true;
+	return 2;
+}
+
+static int condition_proc_write(struct file *file, const char __user *buffer,
+				unsigned long length, void *data)
+{
+	struct condition_variable *var = data;
+	char newval;
+
+	if (length > 0) {
+		if (get_user(newval, buffer) != 0)
+			return -EFAULT;
+		/* Match only on the first character */
+		switch (newval) {
+		case '0':
+			var->enabled = false;
+			break;
+		case '1':
+			var->enabled = true;
+			break;
+		}
+	}
+	return length;
+}
+
+static bool
+condition_mt(const struct sk_buff *skb, struct xt_action_param *par)
+{
+	const struct xt_condition_mtinfo *info = par->matchinfo;
+	const struct condition_variable *var   = info->condvar;
+
+	return var->enabled ^ info->invert;
+}
+
+static int condition_mt_check(const struct xt_mtchk_param *par)
+{
+	struct xt_condition_mtinfo *info = par->matchinfo;
+	struct condition_variable *var;
+	struct condition_net *cond_net = condition_pernet(par->net);
+
+	/* Forbid certain names */
+	if (*info->name == '\0' || *info->name == '.' ||
+	    info->name[sizeof(info->name)-1] != '\0' ||
+	    memchr(info->name, '/', sizeof(info->name)) != NULL) {
+		pr_info("name not allowed or too long: \"%.*s\"\n",
+			(unsigned int)sizeof(info->name), info->name);
+		return -EINVAL;
+	}
+
+	/*
+	 * Let's acquire the lock, check for the condition and add it
+	 * or increase the reference counter.
+	 */
+	mutex_lock(&proc_lock);
+	list_for_each_entry(var, &cond_net->list, list) {
+		if (strcmp(info->name, var->status_proc->name) == 0) {
+			++var->refcount;
+			mutex_unlock(&proc_lock);
+			info->condvar = var;
+			return 0;
+		}
+	}
+
+	/* At this point, we need to allocate a new condition variable. */
+	var = kmalloc(sizeof(struct condition_variable), GFP_KERNEL);
+	if (var == NULL) {
+		mutex_unlock(&proc_lock);
+		return -ENOMEM;
+	}
+
+	/* Create the condition variable's proc file entry. */
+	var->status_proc = create_proc_entry(info->name,
+					     condition_list_perms,
+					     cond_net->proc_dir);
+	if (var->status_proc == NULL) {
+		kfree(var);
+		mutex_unlock(&proc_lock);
+		return -ENOMEM;
+	}
+
+	var->refcount = 1;
+	var->enabled  = false;
+	var->status_proc->data       = var;
+	var->status_proc->read_proc  = condition_proc_read;
+	var->status_proc->write_proc = condition_proc_write;
+	var->status_proc->uid        = condition_uid_perms;
+	var->status_proc->gid        = condition_gid_perms;
+	list_add(&var->list, &cond_net->list);
+	mutex_unlock(&proc_lock);
+	info->condvar = var;
+	return 0;
+}
+
+static void condition_mt_destroy(const struct xt_mtdtor_param *par)
+{
+	const struct xt_condition_mtinfo *info = par->matchinfo;
+	struct condition_variable *var = info->condvar;
+	struct condition_net *cond_net = condition_pernet(par->net);
+
+	mutex_lock(&proc_lock);
+	if (--var->refcount == 0) {
+		list_del(&var->list);
+		/* status_proc may be null in case of ns exit */
+		if (var->status_proc)
+			remove_proc_entry(var->status_proc->name,
+					  cond_net->proc_dir);
+		mutex_unlock(&proc_lock);
+		kfree(var);
+		return;
+	}
+	mutex_unlock(&proc_lock);
+}
+
+static struct xt_match condition_mt_reg __read_mostly = {
+	.name       = "condition",
+	.revision   = 1,
+	.family     = NFPROTO_UNSPEC,
+	.matchsize  = sizeof(struct xt_condition_mtinfo),
+	.match      = condition_mt,
+	.checkentry = condition_mt_check,
+	.destroy    = condition_mt_destroy,
+	.me         = THIS_MODULE,
+};
+
+static const char *const dir_name = "nf_condition";
+
+static int __net_init condnet_mt_init(struct net *net)
+{
+	struct condition_net *cond_net = condition_pernet(net);
+
+	INIT_LIST_HEAD(&cond_net->list);
+
+	cond_net->proc_dir = proc_mkdir(dir_name, net->proc_net);
+
+	return (cond_net->proc_dir == NULL) ? -EACCES : 0;
+}
+
+static void __net_exit condnet_mt_exit(struct net *net)
+{
+	struct condition_net *cond_net = condition_pernet(net);
+	struct condition_variable *var, *tmp;
+
+	mutex_lock(&proc_lock);
+	list_for_each_entry_safe(var, tmp, &cond_net->list, list) {
+		remove_proc_entry(var->status_proc->name,
+				  cond_net->proc_dir);
+		/* set to null so we don't double remove in mt_destroy */
+		var->status_proc = NULL;
+	}
+
+	mutex_unlock(&proc_lock);
+
+	remove_proc_entry(dir_name, net->proc_net);
+}
+
+static struct pernet_operations condition_mt_netops = {
+	.init = condnet_mt_init,
+	.exit = condnet_mt_exit,
+	.id   = &condition_net_id,
+	.size = sizeof(struct condition_net),
+};
+
+static int __init condition_mt_init(void)
+{
+	int ret;
+
+	mutex_init(&proc_lock);
+	ret = xt_register_match(&condition_mt_reg);
+	if (ret < 0)
+		return ret;
+
+	ret = register_pernet_subsys(&condition_mt_netops);
+	if (ret < 0) {
+		xt_unregister_match(&condition_mt_reg);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void __exit condition_mt_exit(void)
+{
+	unregister_pernet_subsys(&condition_mt_netops);
+	xt_unregister_match(&condition_mt_reg);
+}
+
+module_init(condition_mt_init);
+module_exit(condition_mt_exit);
-- 
1.7.0.4

Re: [PATCH RESEND 1/3] netfilter: xtables: inclusion of xt_condition

[Patrick McHardy]

Am 17.08.2010 10:36, schrieb Luciano Coelho:
> diff --git a/net/netfilter/xt_condition.c b/net/netfilter/xt_condition.c
> new file mode 100644
> index 0000000..a78d832
> --- /dev/null
> +++ b/net/netfilter/xt_condition.c
> @@ -0,0 +1,265 @@
> +/*
> + *	"condition" match extension for Xtables
> + *
> + *	Description: This module allows firewall rules to match using
> + *	condition variables available through procfs.
> + *
> + *	Authors:
> + *	Stephane Ouellette <ouellettes [at] videotron ca>, 2002-10-22
> + *	Massimiliano Hofer <max [at] nucleus it>, 2006-05-15
> + *
> + *	This program is free software; you can redistribute it and/or modify it
> + *	under the terms of the GNU General Public License; either version 2
> + *	or 3 of the License, as published by the Free Software Foundation.
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/proc_fs.h>
> +#include <linux/spinlock.h>
> +#include <linux/string.h>
> +#include <linux/version.h>
> +#include <linux/netfilter/x_tables.h>
> +#include <linux/netfilter/xt_condition.h>
> +#include <net/netns/generic.h>
> +#include <asm/uaccess.h>
> +
> +/* Defaults, these can be overridden on the module command-line. */
> +static unsigned int condition_list_perms = S_IRUGO | S_IWUSR;
> +static unsigned int condition_uid_perms = 0;
> +static unsigned int condition_gid_perms = 0;

I'm not sure whether we already discussed this, but this isn't
useful if namespaces are used since the IDs aren't global.

> +
> +MODULE_AUTHOR("Stephane Ouellette <ouellettes@videotron.ca>");
> +MODULE_AUTHOR("Massimiliano Hofer <max@nucleus.it>");
> +MODULE_AUTHOR("Jan Engelhardt <jengelh@medozas.de>");
> +MODULE_DESCRIPTION("Allows rules to match against condition variables");
> +MODULE_LICENSE("GPL");
> +module_param(condition_list_perms, uint, S_IRUSR | S_IWUSR);
> +MODULE_PARM_DESC(condition_list_perms, "default permissions on /proc/net/nf_condition/* files");
> +module_param(condition_uid_perms, uint, S_IRUSR | S_IWUSR);
> +MODULE_PARM_DESC(condition_uid_perms, "default user owner of /proc/net/nf_condition/* files");
> +module_param(condition_gid_perms, uint, S_IRUSR | S_IWUSR);
> +MODULE_PARM_DESC(condition_gid_perms, "default group owner of /proc/net/nf_condition/* files");
> +MODULE_ALIAS("ipt_condition");
> +MODULE_ALIAS("ip6t_condition");
> +

> +/* proc_lock is a user context only semaphore used for write access */
> +/*           to the conditions' list.                               */
> +static DEFINE_MUTEX(proc_lock);

Why is this called proc_lock, as the comment states it protects
the condition variable list? The comment is also misleading since
this is a mutex and not a semaphore. I'd suggest list_mutex or
condition_mutex.

> +static int condition_proc_read(char __user *buffer, char **start, off_t offset,
> +			       int length, int *eof, void *data)
> +{
> +	const struct condition_variable *var = data;
> +
> +	buffer[0] = var->enabled ? '1' : '0';
> +	buffer[1] = '\n';

This is a user buffer, you need copy_to_user(). Also please run
sparse against your code to check for similar errors.

> +	if (length >= 2)
> +		*eof = true;
> +	return 2;
> +}

Re: [PATCH RESEND 1/3] netfilter: xtables: inclusion of xt_condition

[Jan Engelhardt]

On Wednesday 2010-09-15 22:39, Patrick McHardy wrote:
>Am 17.08.2010 10:36, schrieb Luciano Coelho:
>> +/* Defaults, these can be overridden on the module command-line. */
>> +static unsigned int condition_list_perms = S_IRUGO | S_IWUSR;
>> +static unsigned int condition_uid_perms = 0;
>> +static unsigned int condition_gid_perms = 0;
>
>I'm not sure whether we already discussed this, but this isn't
>useful if namespaces are used since the IDs aren't global.

Well it's also done for other xt modules that offer a procfs interface.
It does not really cost much to have this present I'd say.

Re: [PATCH RESEND 1/3] netfilter: xtables: inclusion of xt_condition

[Patrick McHardy]

On 15.09.2010 22:52, Jan Engelhardt wrote:
> 
> On Wednesday 2010-09-15 22:39, Patrick McHardy wrote:
>> Am 17.08.2010 10:36, schrieb Luciano Coelho:
>>> +/* Defaults, these can be overridden on the module command-line. */
>>> +static unsigned int condition_list_perms = S_IRUGO | S_IWUSR;
>>> +static unsigned int condition_uid_perms = 0;
>>> +static unsigned int condition_gid_perms = 0;
>>
>> I'm not sure whether we already discussed this, but this isn't
>> useful if namespaces are used since the IDs aren't global.
> 
> Well it's also done for other xt modules that offer a procfs interface.
> It does not really cost much to have this present I'd say.

They should simply set the permissions for the current namespace.

Re: [PATCH RESEND 1/3] netfilter: xtables: inclusion of xt_condition

[Jan Engelhardt]

On Wednesday 2010-09-15 22:59, Patrick McHardy wrote:
>On 15.09.2010 22:52, Jan Engelhardt wrote:
>> 
>> On Wednesday 2010-09-15 22:39, Patrick McHardy wrote:
>>> Am 17.08.2010 10:36, schrieb Luciano Coelho:
>>>> +/* Defaults, these can be overridden on the module command-line. */
>>>> +static unsigned int condition_list_perms = S_IRUGO | S_IWUSR;
>>>> +static unsigned int condition_uid_perms = 0;
>>>> +static unsigned int condition_gid_perms = 0;
>>>
>>> I'm not sure whether we already discussed this, but this isn't
>>> useful if namespaces are used since the IDs aren't global.
>> 
>> Well it's also done for other xt modules that offer a procfs interface.
>> It does not really cost much to have this present I'd say.
>
>They should simply set the permissions for the current namespace.
>
IIRC, the namespace stuff was going to be a later patch to keep
early patches simpler.

Re: [PATCH RESEND 1/3] netfilter: xtables: inclusion of xt_condition

[Patrick McHardy]

On 15.09.2010 23:09, Jan Engelhardt wrote:
> 
> On Wednesday 2010-09-15 22:59, Patrick McHardy wrote:
>> On 15.09.2010 22:52, Jan Engelhardt wrote:
>>>
>>> On Wednesday 2010-09-15 22:39, Patrick McHardy wrote:
>>>> Am 17.08.2010 10:36, schrieb Luciano Coelho:
>>>>> +/* Defaults, these can be overridden on the module command-line. */
>>>>> +static unsigned int condition_list_perms = S_IRUGO | S_IWUSR;
>>>>> +static unsigned int condition_uid_perms = 0;
>>>>> +static unsigned int condition_gid_perms = 0;
>>>>
>>>> I'm not sure whether we already discussed this, but this isn't
>>>> useful if namespaces are used since the IDs aren't global.
>>>
>>> Well it's also done for other xt modules that offer a procfs interface.
>>> It does not really cost much to have this present I'd say.
>>
>> They should simply set the permissions for the current namespace.
>>
> IIRC, the namespace stuff was going to be a later patch to keep
> early patches simpler.

I'm fine with that as long as its done before merging the patches
upstream.

Re: [PATCH RESEND 1/3] netfilter: xtables: inclusion of xt_condition

[Luciano Coelho]

On Thu, 2010-09-16 at 07:38 +0200, ext Patrick McHardy wrote:
> On 15.09.2010 23:09, Jan Engelhardt wrote:
> > 
> > On Wednesday 2010-09-15 22:59, Patrick McHardy wrote:
> >> On 15.09.2010 22:52, Jan Engelhardt wrote:
> >>>
> >>> On Wednesday 2010-09-15 22:39, Patrick McHardy wrote:
> >>>> Am 17.08.2010 10:36, schrieb Luciano Coelho:
> >>>>> +/* Defaults, these can be overridden on the module command-line. */
> >>>>> +static unsigned int condition_list_perms = S_IRUGO | S_IWUSR;
> >>>>> +static unsigned int condition_uid_perms = 0;
> >>>>> +static unsigned int condition_gid_perms = 0;
> >>>>
> >>>> I'm not sure whether we already discussed this, but this isn't
> >>>> useful if namespaces are used since the IDs aren't global.
> >>>
> >>> Well it's also done for other xt modules that offer a procfs interface.
> >>> It does not really cost much to have this present I'd say.
> >>
> >> They should simply set the permissions for the current namespace.
> >>
> > IIRC, the namespace stuff was going to be a later patch to keep
> > early patches simpler.
> 
> I'm fine with that as long as its done before merging the patches
> upstream.

I'll try to fix this and create a new patch on top of the existing ones,
then I'll send them all as a single patchset.


-- 
Cheers,
Luca.

Re: [PATCH RESEND 1/3] netfilter: xtables: inclusion of xt_condition

[Patrick McHardy]

Am 21.09.2010 22:02, schrieb Luciano Coelho:
> On Thu, 2010-09-16 at 07:38 +0200, ext Patrick McHardy wrote:
>> On 15.09.2010 23:09, Jan Engelhardt wrote:
>>>
>>> On Wednesday 2010-09-15 22:59, Patrick McHardy wrote:
>>>> On 15.09.2010 22:52, Jan Engelhardt wrote:
>>>>>
>>>>> On Wednesday 2010-09-15 22:39, Patrick McHardy wrote:
>>>>>> Am 17.08.2010 10:36, schrieb Luciano Coelho:
>>>>>>> +/* Defaults, these can be overridden on the module command-line. */
>>>>>>> +static unsigned int condition_list_perms = S_IRUGO | S_IWUSR;
>>>>>>> +static unsigned int condition_uid_perms = 0;
>>>>>>> +static unsigned int condition_gid_perms = 0;
>>>>>>
>>>>>> I'm not sure whether we already discussed this, but this isn't
>>>>>> useful if namespaces are used since the IDs aren't global.
>>>>>
>>>>> Well it's also done for other xt modules that offer a procfs interface.
>>>>> It does not really cost much to have this present I'd say.
>>>>
>>>> They should simply set the permissions for the current namespace.
>>>>
>>> IIRC, the namespace stuff was going to be a later patch to keep
>>> early patches simpler.
>>
>> I'm fine with that as long as its done before merging the patches
>> upstream.
> 
> I'll try to fix this and create a new patch on top of the existing ones,
> then I'll send them all as a single patchset.

Sounds good, thanks.

Re: [PATCH RESEND 1/3] netfilter: xtables: inclusion of xt_condition

[Luciano Coelho]

On Wed, 2010-09-15 at 22:39 +0200, ext Patrick McHardy wrote:
> Am 17.08.2010 10:36, schrieb Luciano Coelho:
> > diff --git a/net/netfilter/xt_condition.c b/net/netfilter/xt_condition.c
> > new file mode 100644
> > index 0000000..a78d832
> > --- /dev/null
> > +++ b/net/netfilter/xt_condition.c
> > @@ -0,0 +1,265 @@
> > +/*
> > + *	"condition" match extension for Xtables
> > + *
> > + *	Description: This module allows firewall rules to match using
> > + *	condition variables available through procfs.
> > + *
> > + *	Authors:
> > + *	Stephane Ouellette <ouellettes [at] videotron ca>, 2002-10-22
> > + *	Massimiliano Hofer <max [at] nucleus it>, 2006-05-15
> > + *
> > + *	This program is free software; you can redistribute it and/or modify it
> > + *	under the terms of the GNU General Public License; either version 2
> > + *	or 3 of the License, as published by the Free Software Foundation.
> > + */
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +#include <linux/kernel.h>
> > +#include <linux/list.h>
> > +#include <linux/module.h>
> > +#include <linux/proc_fs.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/string.h>
> > +#include <linux/version.h>
> > +#include <linux/netfilter/x_tables.h>
> > +#include <linux/netfilter/xt_condition.h>
> > +#include <net/netns/generic.h>
> > +#include <asm/uaccess.h>
> > +
> > +/* Defaults, these can be overridden on the module command-line. */
> > +static unsigned int condition_list_perms = S_IRUGO | S_IWUSR;
> > +static unsigned int condition_uid_perms = 0;
> > +static unsigned int condition_gid_perms = 0;
> 
> I'm not sure whether we already discussed this, but this isn't
> useful if namespaces are used since the IDs aren't global.

As Jan suggested, I think it would be better to include it like this,
since other modules already do similar things, and then I'll later send
a new patch with support for multiple namespaces.


> > +
> > +MODULE_AUTHOR("Stephane Ouellette <ouellettes@videotron.ca>");
> > +MODULE_AUTHOR("Massimiliano Hofer <max@nucleus.it>");
> > +MODULE_AUTHOR("Jan Engelhardt <jengelh@medozas.de>");
> > +MODULE_DESCRIPTION("Allows rules to match against condition variables");
> > +MODULE_LICENSE("GPL");
> > +module_param(condition_list_perms, uint, S_IRUSR | S_IWUSR);
> > +MODULE_PARM_DESC(condition_list_perms, "default permissions on /proc/net/nf_condition/* files");
> > +module_param(condition_uid_perms, uint, S_IRUSR | S_IWUSR);
> > +MODULE_PARM_DESC(condition_uid_perms, "default user owner of /proc/net/nf_condition/* files");
> > +module_param(condition_gid_perms, uint, S_IRUSR | S_IWUSR);
> > +MODULE_PARM_DESC(condition_gid_perms, "default group owner of /proc/net/nf_condition/* files");
> > +MODULE_ALIAS("ipt_condition");
> > +MODULE_ALIAS("ip6t_condition");
> > +
> 
> > +/* proc_lock is a user context only semaphore used for write access */
> > +/*           to the conditions' list.                               */
> > +static DEFINE_MUTEX(proc_lock);
> 
> Why is this called proc_lock, as the comment states it protects
> the condition variable list? The comment is also misleading since
> this is a mutex and not a semaphore. I'd suggest list_mutex or
> condition_mutex.

I'll fix this.


> > +static int condition_proc_read(char __user *buffer, char **start, off_t offset,
> > +			       int length, int *eof, void *data)
> > +{
> > +	const struct condition_variable *var = data;
> > +
> > +	buffer[0] = var->enabled ? '1' : '0';
> > +	buffer[1] = '\n';
> 
> This is a user buffer, you need copy_to_user(). Also please run
> sparse against your code to check for similar errors.

Oooff, I actually also noticed this when integrating into our internal
tree.  I'll fix this and run sparse properly before resending.


-- 
Cheers,
Luca.

[PATCH RESEND 2/3] netfilter: xt_condition: change the value from boolean to u32

[Luciano Coelho]

Previously the condition match was using boolean values for the condition,
which is a bit limited, so this patch changes the value to a u32.

This is not a problem at the moment, since only userspace can set the
condition and it can do more advanced checks before setting the value.  But
when the condition target gets implemented, having a u32 value will make it
much more flexible.

Signed-off-by: Luciano Coelho <luciano.coelho@nokia.com>
---
 include/linux/netfilter/xt_condition.h |    3 +-
 net/netfilter/xt_condition.c           |   54 +++++++++++++++++---------------
 2 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/include/linux/netfilter/xt_condition.h b/include/linux/netfilter/xt_condition.h
index 4faf3ca..c4fe899 100644
--- a/include/linux/netfilter/xt_condition.h
+++ b/include/linux/netfilter/xt_condition.h
@@ -4,8 +4,9 @@
 #include <linux/types.h>
 
 struct xt_condition_mtinfo {
-	char name[31];
+	char name[27];
 	__u8 invert;
+	__u32 value;
 
 	/* Used internally by the kernel */
 	void *condvar __attribute__((aligned(8)));
diff --git a/net/netfilter/xt_condition.c b/net/netfilter/xt_condition.c
index a78d832..cc9ada9 100644
--- a/net/netfilter/xt_condition.c
+++ b/net/netfilter/xt_condition.c
@@ -48,7 +48,7 @@ struct condition_variable {
 	struct list_head list;
 	struct proc_dir_entry *status_proc;
 	unsigned int refcount;
-	bool enabled;
+	u32 value;
 };
 
 struct condition_net {
@@ -71,32 +71,36 @@ static int condition_proc_read(char __user *buffer, char **start, off_t offset,
 {
 	const struct condition_variable *var = data;
 
-	buffer[0] = var->enabled ? '1' : '0';
-	buffer[1] = '\n';
-	if (length >= 2)
-		*eof = true;
-	return 2;
+	return snprintf(buffer, length, "%u\n", var->value);
 }
 
-static int condition_proc_write(struct file *file, const char __user *buffer,
+static int condition_proc_write(struct file *file, const char __user *input,
 				unsigned long length, void *data)
 {
 	struct condition_variable *var = data;
-	char newval;
-
-	if (length > 0) {
-		if (get_user(newval, buffer) != 0)
-			return -EFAULT;
-		/* Match only on the first character */
-		switch (newval) {
-		case '0':
-			var->enabled = false;
-			break;
-		case '1':
-			var->enabled = true;
-			break;
-		}
-	}
+	/* the longest possible string is MAX_UINT in octal */
+	char buf[sizeof("+037777777777")];
+	unsigned long long value;
+
+	if (length == 0)
+		return 0;
+
+	if (length > sizeof(buf))
+		return -EINVAL;
+
+	if (copy_from_user(buf, input, length) != 0)
+		return -EFAULT;
+
+	buf[length - 1] = '\0';
+
+	if (strict_strtoull(buf, 0, &value) != 0)
+		return -EINVAL;
+
+	if (value > (u32) value)
+		return -ERANGE;
+
+	var->value = value;
+
 	return length;
 }
 
@@ -106,7 +110,7 @@ condition_mt(const struct sk_buff *skb, struct xt_action_param *par)
 	const struct xt_condition_mtinfo *info = par->matchinfo;
 	const struct condition_variable *var   = info->condvar;
 
-	return var->enabled ^ info->invert;
+	return (var->value == info->value) ^ info->invert;
 }
 
 static int condition_mt_check(const struct xt_mtchk_param *par)
@@ -156,7 +160,7 @@ static int condition_mt_check(const struct xt_mtchk_param *par)
 	}
 
 	var->refcount = 1;
-	var->enabled  = false;
+	var->value = 0;
 	var->status_proc->data       = var;
 	var->status_proc->read_proc  = condition_proc_read;
 	var->status_proc->write_proc = condition_proc_write;
@@ -190,7 +194,7 @@ static void condition_mt_destroy(const struct xt_mtdtor_param *par)
 
 static struct xt_match condition_mt_reg __read_mostly = {
 	.name       = "condition",
-	.revision   = 1,
+	.revision   = 2,
 	.family     = NFPROTO_UNSPEC,
 	.matchsize  = sizeof(struct xt_condition_mtinfo),
 	.match      = condition_mt,
-- 
1.7.0.4

Re: [PATCH RESEND 2/3] netfilter: xt_condition: change the value from boolean to u32

[Patrick McHardy]

Am 17.08.2010 10:36, schrieb Luciano Coelho:
>  static struct xt_match condition_mt_reg __read_mostly = {
>  	.name       = "condition",
> -	.revision   = 1,
> +	.revision   = 2,

Is this previous version actually compatible with anything that has
ever been released?

Re: [PATCH RESEND 2/3] netfilter: xt_condition: change the value from boolean to u32

[Jan Engelhardt]

On Wednesday 2010-09-15 22:41, Patrick McHardy wrote:

>Am 17.08.2010 10:36, schrieb Luciano Coelho:
>>  static struct xt_match condition_mt_reg __read_mostly = {
>>  	.name       = "condition",
>> -	.revision   = 1,
>> +	.revision   = 2,
>
>Is this previous version actually compatible with anything that has
>ever been released?

Yes, rev1 is used by Xtables-addons.

Re: [PATCH RESEND 2/3] netfilter: xt_condition: change the value from boolean to u32

[Luciano Coelho]

On Wed, 2010-09-15 at 22:46 +0200, ext Jan Engelhardt wrote:
> On Wednesday 2010-09-15 22:41, Patrick McHardy wrote:
> 
> >Am 17.08.2010 10:36, schrieb Luciano Coelho:
> >>  static struct xt_match condition_mt_reg __read_mostly = {
> >>  	.name       = "condition",
> >> -	.revision   = 1,
> >> +	.revision   = 2,
> >
> >Is this previous version actually compatible with anything that has
> >ever been released?
> 
> Yes, rev1 is used by Xtables-addons.

Indeed.  It was Jan who recommended this to be changed to 2 to avoid
conflict with Xtables-addons.


-- 
Cheers,
Luca.

[PATCH RESEND 3/3] netfilter: xt_condition: add condition target support

[Luciano Coelho]

This patch implements a condition target to the xt_condition module,
which let's the user set netfilter rules that change the variables
used by the condition match.  Originally, the condition match only
allowed the variable to be changed via procfs.  This new target makes
it easy to change the condition value depending on the rules set.

Signed-off-by: Luciano Coelho <luciano.coelho@nokia.com>
---
 include/linux/netfilter/xt_condition.h |   13 ++-
 net/netfilter/Kconfig                  |   19 ++--
 net/netfilter/Makefile                 |    2 +-
 net/netfilter/xt_condition.c           |  194 +++++++++++++++++++++++---------
 4 files changed, 165 insertions(+), 63 deletions(-)

diff --git a/include/linux/netfilter/xt_condition.h b/include/linux/netfilter/xt_condition.h
index c4fe899..946d43a 100644
--- a/include/linux/netfilter/xt_condition.h
+++ b/include/linux/netfilter/xt_condition.h
@@ -3,8 +3,10 @@
 
 #include <linux/types.h>
 
+#define XT_CONDITION_MAX_NAME_SIZE 27
+
 struct xt_condition_mtinfo {
-	char name[27];
+	char name[XT_CONDITION_MAX_NAME_SIZE];
 	__u8 invert;
 	__u32 value;
 
@@ -12,4 +14,13 @@ struct xt_condition_mtinfo {
 	void *condvar __attribute__((aligned(8)));
 };
 
+struct condition_tginfo {
+	char name[XT_CONDITION_MAX_NAME_SIZE];
+	__u8 padding;
+	__u32 value;
+
+	/* Used internally by the kernel */
+	void *condvar __attribute__((aligned(8)));
+};
+
 #endif /* _XT_CONDITION_H */
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 5044dd6..d9a17eb 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -310,6 +310,17 @@ config NETFILTER_XT_MARK
 	"Use netfilter MARK value as routing key") and can also be used by
 	other subsystems to change their behavior.
 
+config NETFILTER_XT_CONDITION
+       tristate '"condition" match and target support'
+       depends on NETFILTER_ADVANCED
+       depends on PROC_FS
+       ---help---
+       This option adds the "CONDITION" target and "condition" match.
+
+       It allows you to match rules against condition variables
+       stored in the /proc/net/nf_condition directory. It also allows
+       you to set the variables using the target.
+
 config NETFILTER_XT_CONNMARK
 	tristate 'ctmark target and match support'
 	depends on NF_CONNTRACK
@@ -621,14 +632,6 @@ config NETFILTER_XT_MATCH_COMMENT
 	  If you want to compile it as a module, say M here and read
 	  <file:Documentation/kbuild/modules.txt>.  If unsure, say `N'.
 
-config NETFILTER_XT_MATCH_CONDITION
-	tristate '"condition" match support'
-	depends on NETFILTER_ADVANCED
-	depends on PROC_FS
-	---help---
-	This option allows you to match firewall rules against condition
-	variables stored in the /proc/net/nf_condition directory.
-
 config NETFILTER_XT_MATCH_CONNBYTES
 	tristate  '"connbytes" per-connection counter match support'
 	depends on NF_CONNTRACK
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index bbf72bb..146a05f 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_NETFILTER_XTABLES) += x_tables.o xt_tcpudp.o
 # combos
 obj-$(CONFIG_NETFILTER_XT_MARK) += xt_mark.o
 obj-$(CONFIG_NETFILTER_XT_CONNMARK) += xt_connmark.o
+obj-$(CONFIG_NETFILTER_XT_CONDITION) += xt_condition.o
 
 # targets
 obj-$(CONFIG_NETFILTER_XT_TARGET_CHECKSUM) += xt_CHECKSUM.o
@@ -67,7 +68,6 @@ obj-$(CONFIG_NETFILTER_XT_TARGET_IDLETIMER) += xt_IDLETIMER.o
 # matches
 obj-$(CONFIG_NETFILTER_XT_MATCH_CLUSTER) += xt_cluster.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_COMMENT) += xt_comment.o
-obj-$(CONFIG_NETFILTER_XT_MATCH_CONDITION) += xt_condition.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_CONNBYTES) += xt_connbytes.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_CONNLIMIT) += xt_connlimit.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_CONNTRACK) += xt_conntrack.o
diff --git a/net/netfilter/xt_condition.c b/net/netfilter/xt_condition.c
index cc9ada9..06205aa 100644
--- a/net/netfilter/xt_condition.c
+++ b/net/netfilter/xt_condition.c
@@ -2,15 +2,20 @@
  *	"condition" match extension for Xtables
  *
  *	Description: This module allows firewall rules to match using
- *	condition variables available through procfs.
+ *	condition variables available through procfs.  It also allows
+ *	target rules to set the condition variable.
  *
  *	Authors:
  *	Stephane Ouellette <ouellettes [at] videotron ca>, 2002-10-22
  *	Massimiliano Hofer <max [at] nucleus it>, 2006-05-15
+ *	Luciano Coelho <luciano.coelho@nokia.com>, 2010-08-11
  *
  *	This program is free software; you can redistribute it and/or modify it
  *	under the terms of the GNU General Public License; either version 2
  *	or 3 of the License, as published by the Free Software Foundation.
+ *
+ *	Portion Copyright 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *	File modified on 2010-08-11.
  */
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include <linux/kernel.h>
@@ -33,7 +38,8 @@ static unsigned int condition_gid_perms = 0;
 MODULE_AUTHOR("Stephane Ouellette <ouellettes@videotron.ca>");
 MODULE_AUTHOR("Massimiliano Hofer <max@nucleus.it>");
 MODULE_AUTHOR("Jan Engelhardt <jengelh@medozas.de>");
-MODULE_DESCRIPTION("Allows rules to match against condition variables");
+MODULE_AUTHOR("Luciano Coelho <luciano.coelho@nokia.com>");
+MODULE_DESCRIPTION("Allows rules to set and match condition variables");
 MODULE_LICENSE("GPL");
 module_param(condition_list_perms, uint, S_IRUSR | S_IWUSR);
 MODULE_PARM_DESC(condition_list_perms, "default permissions on /proc/net/nf_condition/* files");
@@ -104,29 +110,11 @@ static int condition_proc_write(struct file *file, const char __user *input,
 	return length;
 }
 
-static bool
-condition_mt(const struct sk_buff *skb, struct xt_action_param *par)
-{
-	const struct xt_condition_mtinfo *info = par->matchinfo;
-	const struct condition_variable *var   = info->condvar;
-
-	return (var->value == info->value) ^ info->invert;
-}
-
-static int condition_mt_check(const struct xt_mtchk_param *par)
+static struct
+condition_variable *xt_condition_insert(const char *name,
+					struct condition_net *cond_net)
 {
-	struct xt_condition_mtinfo *info = par->matchinfo;
 	struct condition_variable *var;
-	struct condition_net *cond_net = condition_pernet(par->net);
-
-	/* Forbid certain names */
-	if (*info->name == '\0' || *info->name == '.' ||
-	    info->name[sizeof(info->name)-1] != '\0' ||
-	    memchr(info->name, '/', sizeof(info->name)) != NULL) {
-		pr_info("name not allowed or too long: \"%.*s\"\n",
-			(unsigned int)sizeof(info->name), info->name);
-		return -EINVAL;
-	}
 
 	/*
 	 * Let's acquire the lock, check for the condition and add it
@@ -134,29 +122,24 @@ static int condition_mt_check(const struct xt_mtchk_param *par)
 	 */
 	mutex_lock(&proc_lock);
 	list_for_each_entry(var, &cond_net->list, list) {
-		if (strcmp(info->name, var->status_proc->name) == 0) {
+		if (strcmp(name, var->status_proc->name) == 0) {
 			++var->refcount;
-			mutex_unlock(&proc_lock);
-			info->condvar = var;
-			return 0;
+			goto out;
 		}
 	}
 
 	/* At this point, we need to allocate a new condition variable. */
 	var = kmalloc(sizeof(struct condition_variable), GFP_KERNEL);
-	if (var == NULL) {
-		mutex_unlock(&proc_lock);
-		return -ENOMEM;
-	}
+	if (var == NULL)
+		goto out;
 
 	/* Create the condition variable's proc file entry. */
-	var->status_proc = create_proc_entry(info->name,
-					     condition_list_perms,
+	var->status_proc = create_proc_entry(name, condition_list_perms,
 					     cond_net->proc_dir);
 	if (var->status_proc == NULL) {
 		kfree(var);
-		mutex_unlock(&proc_lock);
-		return -ENOMEM;
+		var = NULL;
+		goto out;
 	}
 
 	var->refcount = 1;
@@ -167,17 +150,14 @@ static int condition_mt_check(const struct xt_mtchk_param *par)
 	var->status_proc->uid        = condition_uid_perms;
 	var->status_proc->gid        = condition_gid_perms;
 	list_add(&var->list, &cond_net->list);
+out:
 	mutex_unlock(&proc_lock);
-	info->condvar = var;
-	return 0;
+	return var;
 }
 
-static void condition_mt_destroy(const struct xt_mtdtor_param *par)
+static void xt_condition_put(struct condition_variable *var,
+			     struct condition_net *cond_net)
 {
-	const struct xt_condition_mtinfo *info = par->matchinfo;
-	struct condition_variable *var = info->condvar;
-	struct condition_net *cond_net = condition_pernet(par->net);
-
 	mutex_lock(&proc_lock);
 	if (--var->refcount == 0) {
 		list_del(&var->list);
@@ -192,6 +172,106 @@ static void condition_mt_destroy(const struct xt_mtdtor_param *par)
 	mutex_unlock(&proc_lock);
 }
 
+static bool
+condition_mt(const struct sk_buff *skb, struct xt_action_param *par)
+{
+	const struct xt_condition_mtinfo *info = par->matchinfo;
+	const struct condition_variable *var   = info->condvar;
+
+	return (var->value == info->value) ^ info->invert;
+}
+
+static int condition_mt_check(const struct xt_mtchk_param *par)
+{
+	struct xt_condition_mtinfo *info = par->matchinfo;
+	struct condition_variable *var;
+	struct condition_net *cond_net = condition_pernet(par->net);
+
+	/* Forbid certain names */
+	if (*info->name == '\0' || *info->name == '.' ||
+	    info->name[sizeof(info->name)-1] != '\0' ||
+	    memchr(info->name, '/', sizeof(info->name)) != NULL) {
+		pr_info("name not allowed or too long: \"%.*s\"\n",
+			(unsigned int)sizeof(info->name), info->name);
+		return -EINVAL;
+	}
+
+	var = xt_condition_insert(info->name, cond_net);
+	if (var == NULL)
+		return -ENOMEM;
+
+	info->condvar = var;
+	return 0;
+}
+
+static void condition_mt_destroy(const struct xt_mtdtor_param *par)
+{
+	const struct xt_condition_mtinfo *info = par->matchinfo;
+	struct condition_net *cond_net = condition_pernet(par->net);
+
+	xt_condition_put(info->condvar, cond_net);
+}
+
+static unsigned int condition_tg_target(struct sk_buff *skb,
+					const struct xt_action_param *par)
+{
+	const struct condition_tginfo *info = par->targinfo;
+	struct condition_variable *var = info->condvar;
+
+	pr_debug("setting condition %s, value %d\n",
+		 info->name, info->value);
+
+	var->value = info->value;
+
+	return XT_CONTINUE;
+}
+
+static int condition_tg_checkentry(const struct xt_tgchk_param *par)
+{
+	struct condition_tginfo *info = par->targinfo;
+	struct condition_variable *var;
+	struct condition_net *cond_net = condition_pernet(par->net);
+
+	pr_debug("checkentry %s\n", info->name);
+
+	/* Forbid certain names */
+	if (*info->name == '\0' || *info->name == '.' ||
+	    info->name[sizeof(info->name)-1] != '\0' ||
+	    memchr(info->name, '/', sizeof(info->name)) != NULL) {
+		pr_info("name not allowed or too long: \"%.*s\"\n",
+			(unsigned int)sizeof(info->name), info->name);
+		return -EINVAL;
+	}
+
+	var = xt_condition_insert(info->name, cond_net);
+	if (var == NULL)
+		return -ENOMEM;
+
+	info->condvar = var;
+	return 0;
+}
+
+static void condition_tg_destroy(const struct xt_tgdtor_param *par)
+{
+	const struct condition_tginfo *info = par->targinfo;
+	struct condition_net *cond_net = condition_pernet(par->net);
+
+	pr_debug("destroy %s\n", info->name);
+
+	xt_condition_put(info->condvar, cond_net);
+}
+
+static struct xt_target condition_tg_reg __read_mostly = {
+	.name           = "CONDITION",
+	.revision       = 0,
+	.family         = NFPROTO_UNSPEC,
+	.target         = condition_tg_target,
+	.targetsize     = sizeof(struct condition_tginfo),
+	.checkentry     = condition_tg_checkentry,
+	.destroy        = condition_tg_destroy,
+	.me             = THIS_MODULE,
+};
+
 static struct xt_match condition_mt_reg __read_mostly = {
 	.name       = "condition",
 	.revision   = 2,
@@ -205,7 +285,7 @@ static struct xt_match condition_mt_reg __read_mostly = {
 
 static const char *const dir_name = "nf_condition";
 
-static int __net_init condnet_mt_init(struct net *net)
+static int __net_init condnet_init(struct net *net)
 {
 	struct condition_net *cond_net = condition_pernet(net);
 
@@ -216,7 +296,7 @@ static int __net_init condnet_mt_init(struct net *net)
 	return (cond_net->proc_dir == NULL) ? -EACCES : 0;
 }
 
-static void __net_exit condnet_mt_exit(struct net *net)
+static void __net_exit condnet_exit(struct net *net)
 {
 	struct condition_net *cond_net = condition_pernet(net);
 	struct condition_variable *var, *tmp;
@@ -234,14 +314,14 @@ static void __net_exit condnet_mt_exit(struct net *net)
 	remove_proc_entry(dir_name, net->proc_net);
 }
 
-static struct pernet_operations condition_mt_netops = {
-	.init = condnet_mt_init,
-	.exit = condnet_mt_exit,
+static struct pernet_operations condition_netops = {
+	.init = condnet_init,
+	.exit = condnet_exit,
 	.id   = &condition_net_id,
 	.size = sizeof(struct condition_net),
 };
 
-static int __init condition_mt_init(void)
+static int __init condition_init(void)
 {
 	int ret;
 
@@ -250,8 +330,15 @@ static int __init condition_mt_init(void)
 	if (ret < 0)
 		return ret;
 
-	ret = register_pernet_subsys(&condition_mt_netops);
+	ret =  xt_register_target(&condition_tg_reg);
+	if (ret < 0) {
+		xt_unregister_match(&condition_mt_reg);
+		return ret;
+	}
+
+	ret = register_pernet_subsys(&condition_netops);
 	if (ret < 0) {
+		xt_unregister_target(&condition_tg_reg);
 		xt_unregister_match(&condition_mt_reg);
 		return ret;
 	}
@@ -259,11 +346,12 @@ static int __init condition_mt_init(void)
 	return 0;
 }
 
-static void __exit condition_mt_exit(void)
+static void __exit condition_exit(void)
 {
-	unregister_pernet_subsys(&condition_mt_netops);
+	unregister_pernet_subsys(&condition_netops);
 	xt_unregister_match(&condition_mt_reg);
+	xt_unregister_target(&condition_tg_reg);
 }
 
-module_init(condition_mt_init);
-module_exit(condition_mt_exit);
+module_init(condition_init);
+module_exit(condition_exit);
-- 
1.7.0.4