Skip to contentSkip to conversation

netfilter: ctnetlink: helper modules load-on-demand support

PATCH Applied Pablo Neira Ayuso 2 participants 4 messages Nov 18, 2008

Subsystems (2)
Networking netfilter
lore
Share & tools
lore

Pablo Neira Ayuso

author maintainer
#0 lore raw

PATCH 1/2 netfilter: ctnetlink: helper modules load-on-demand support

This patch adds module loading for helpers via ctnetlink.

* Creation path: We support explicit and implicit helper assignation. For the explicit case, we try to load the module. If the module is correctly loaded and the helper is present, we return EAGAIN to re-start the creation. Otherwise, we return EOPNOTSUPP.
* Update path: release the spin lock, load the module and check. If it is present, then return EAGAIN to re-start the update.

This patch provides a refactorized function to lookup-and-set the connection tracking helper. The function removes the exported symbol __nf_ct_helper_find as it has not clients anymore.

Signed-off-by Pablo Neira Ayuso <[email protected]>

---

 include/net/netfilter/nf_conntrack_helper.h |    5 +-
 net/netfilter/nf_conntrack_core.c           |   28 +----------
 net/netfilter/nf_conntrack_helper.c         |   32 ++++++++++++
 net/netfilter/nf_conntrack_netlink.c        |   70 ++++++++++++++++++++++++---
 4 files changed, 95 insertions(+), 40 deletions(-)
include/net/netfilter/nf_conntrack_helper.h +2 -3
index f8060ab..66d65a7 100644
@@ -39,9 +39,6 @@ struct nf_conntrack_helper
 };
 
 extern struct nf_conntrack_helper *
-__nf_ct_helper_find(const struct nf_conntrack_tuple *tuple);
-
-extern struct nf_conntrack_helper *
 __nf_conntrack_helper_find_byname(const char *name);
 
 extern int nf_conntrack_helper_register(struct nf_conntrack_helper *);

@@ -49,6 +46,8 @@ extern void nf_conntrack_helper_unregister(struct nf_conntrack_helper *);
 
 extern struct nf_conn_help *nf_ct_helper_ext_add(struct nf_conn *ct, gfp_t gfp);
 
+extern int __nf_ct_try_assign_helper(struct nf_conn *ct, gfp_t flags);
+
 static inline struct nf_conn_help *nfct_help(const struct nf_conn *ct)
 {
 	return nf_ct_ext_find(ct, NF_CT_EXT_HELPER);
net/netfilter/nf_conntrack_core.c +2 -26
index 622d7c6..1e649fb 100644
@@ -588,14 +588,7 @@ init_conntrack(struct net *net,
 		nf_conntrack_get(&ct->master->ct_general);
 		NF_CT_STAT_INC(net, expect_new);
 	} else {
-		struct nf_conntrack_helper *helper;
-
-		helper = __nf_ct_helper_find(&repl_tuple);
-		if (helper) {
-			help = nf_ct_helper_ext_add(ct, GFP_ATOMIC);
-			if (help)
-				rcu_assign_pointer(help->helper, helper);
-		}
+		__nf_ct_try_assign_helper(ct, GFP_ATOMIC);
 		NF_CT_STAT_INC(net, new);
 	}
 

@@ -772,7 +765,6 @@ void nf_conntrack_alter_reply(struct nf_conn *ct,
 			      const struct nf_conntrack_tuple *newreply)
 {
 	struct nf_conn_help *help = nfct_help(ct);
-	struct nf_conntrack_helper *helper;
 
 	/* Should be unconfirmed, so not in hash table yet */
 	NF_CT_ASSERT(!nf_ct_is_confirmed(ct));

@@ -785,23 +777,7 @@ void nf_conntrack_alter_reply(struct nf_conn *ct,
 		return;
 
 	rcu_read_lock();
-	helper = __nf_ct_helper_find(newreply);
-	if (helper == NULL) {
-		if (help)
-			rcu_assign_pointer(help->helper, NULL);
-		goto out;
-	}
-
-	if (help == NULL) {
-		help = nf_ct_helper_ext_add(ct, GFP_ATOMIC);
-		if (help == NULL)
-			goto out;
-	} else {
-		memset(&help->help, 0, sizeof(help->help));
-	}
-
-	rcu_assign_pointer(help->helper, helper);
-out:
+	__nf_ct_try_assign_helper(ct, GFP_ATOMIC);
 	rcu_read_unlock();
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_alter_reply);
net/netfilter/nf_conntrack_helper.c +30 -2
index c39b6a9..a51bdac 100644
@@ -45,7 +45,7 @@ static unsigned int helper_hash(const struct nf_conntrack_tuple *tuple)
 		(__force __u16)tuple->src.u.all) % nf_ct_helper_hsize;
 }
 
-struct nf_conntrack_helper *
+static struct nf_conntrack_helper *
 __nf_ct_helper_find(const struct nf_conntrack_tuple *tuple)
 {
 	struct nf_conntrack_helper *helper;

@@ -63,7 +63,6 @@ __nf_ct_helper_find(const struct nf_conntrack_tuple *tuple)
 	}
 	return NULL;
 }
-EXPORT_SYMBOL_GPL(__nf_ct_helper_find);
 
 struct nf_conntrack_helper *
 __nf_conntrack_helper_find_byname(const char *name)

@@ -95,6 +94,35 @@ struct nf_conn_help *nf_ct_helper_ext_add(struct nf_conn *ct, gfp_t gfp)
 }
 EXPORT_SYMBOL_GPL(nf_ct_helper_ext_add);
 
+int __nf_ct_try_assign_helper(struct nf_conn *ct, gfp_t flags)
+{
+	int ret = 0;
+	struct nf_conntrack_helper *helper;
+	struct nf_conn_help *help = nfct_help(ct);
+
+	helper = __nf_ct_helper_find(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
+	if (helper == NULL) {
+		if (help)
+			rcu_assign_pointer(help->helper, NULL);
+		goto out;
+	}
+
+	if (help == NULL) {
+		help = nf_ct_helper_ext_add(ct, flags);
+		if (help == NULL) {
+			ret = -ENOMEM;
+			goto out;
+		}
+	} else {
+		memset(&help->help, 0, sizeof(help->help));
+	}
+
+	rcu_assign_pointer(help->helper, helper);
+out:
+	return ret;
+}
+EXPORT_SYMBOL_GPL(__nf_ct_try_assign_helper);
+
 static inline int unhelp(struct nf_conntrack_tuple_hash *i,
 			 const struct nf_conntrack_helper *me)
 {
net/netfilter/nf_conntrack_netlink.c +61 -9

92 lines not on this page · raw message

author maintainer
#1 lore raw

PATCH 2/2 netfilter: ctnetlink: deliver events for conntracks changed from userspace

As for now, the creation and update of conntracks via ctnetlink do not propagate an event to userspace. This can result in inconsistent situations if several userspace processes modify the connection tracking table by means of ctnetlink at the same time. Specifically, using the conntrack command line tool and conntrackd at the same time can trigger unconsistencies.

This patch also modifies the event cache infrastructure to pass the process PID and the ECHO flag to nfnetlink_send() to report back to userspace if the process that triggered the change needs so. Based on a suggestion from Patrick McHardy.

Signed-off-by Pablo Neira Ayuso <[email protected]>

---

 include/net/netfilter/nf_conntrack.h        |    2 -
 include/net/netfilter/nf_conntrack_ecache.h |   57 ++++++++++++++++-
 include/net/netfilter/nf_conntrack_expect.h |    2 +
 net/netfilter/nf_conntrack_core.c           |   25 ++++++--
 net/netfilter/nf_conntrack_ecache.c         |   14 +++-
 net/netfilter/nf_conntrack_expect.c         |   43 +++++++++++--
 net/netfilter/nf_conntrack_netlink.c        |   88 ++++++++++++++++++++++-----
 7 files changed, 197 insertions(+), 34 deletions(-)
include/net/netfilter/nf_conntrack.h +1 -1
index f11255e..2e0c536 100644
@@ -199,7 +199,7 @@ __nf_conntrack_find(struct net *net, const struct nf_conntrack_tuple *tuple);
 
 extern void nf_conntrack_hash_insert(struct nf_conn *ct);
 
-extern void nf_conntrack_flush(struct net *net);
+extern void nf_conntrack_flush(struct net *net, u32 pid, int report);
 
 extern bool nf_ct_get_tuplepr(const struct sk_buff *skb,
 			      unsigned int nhoff, u_int16_t l3num,
include/net/netfilter/nf_conntrack_ecache.h +53 -4

90 lines not on this page · raw message

include/net/netfilter/nf_conntrack_expect.h +2 -0
index 37a7fc1..ab17a15 100644
@@ -100,6 +100,8 @@ void nf_ct_expect_init(struct nf_conntrack_expect *, unsigned int, u_int8_t,
 		       u_int8_t, const __be16 *, const __be16 *);
 void nf_ct_expect_put(struct nf_conntrack_expect *exp);
 int nf_ct_expect_related(struct nf_conntrack_expect *expect);
+int nf_ct_expect_related_report(struct nf_conntrack_expect *expect, 
+				u32 pid, int report);
 
 #endif /*_NF_CONNTRACK_EXPECT_H*/
net/netfilter/nf_conntrack_core.c +21 -4
index 1e649fb..dc3fea0 100644
@@ -181,7 +181,8 @@ destroy_conntrack(struct nf_conntrack *nfct)
 	NF_CT_ASSERT(atomic_read(&nfct->use) == 0);
 	NF_CT_ASSERT(!timer_pending(&ct->timeout));
 
-	nf_conntrack_event(IPCT_DESTROY, ct);
+	if (!test_bit(IPS_DYING_BIT, &ct->status))
+		nf_conntrack_event(IPCT_DESTROY, ct);
 	set_bit(IPS_DYING_BIT, &ct->status);
 
 	/* To make sure we don't get any weird locking issues here:

@@ -972,8 +973,20 @@ void nf_ct_iterate_cleanup(struct net *net,
 }
 EXPORT_SYMBOL_GPL(nf_ct_iterate_cleanup);
 
+struct __nf_ct_flush_report {
+	u32 pid;
+	int report;
+};
+
 static int kill_all(struct nf_conn *i, void *data)
 {
+	struct __nf_ct_flush_report *fr = (struct __nf_ct_flush_report *)data;
+
+	/* get_next_corpse sets the dying bit for us */
+	nf_conntrack_event_report(IPCT_DESTROY,
+				  i,
+				  fr->pid,
+				  fr->report);
 	return 1;
 }
 

@@ -987,9 +1000,13 @@ void nf_ct_free_hashtable(struct hlist_head *hash, int vmalloced, unsigned int s
 }
 EXPORT_SYMBOL_GPL(nf_ct_free_hashtable);
 
-void nf_conntrack_flush(struct net *net)
+void nf_conntrack_flush(struct net *net, u32 pid, int report)
 {
-	nf_ct_iterate_cleanup(net, kill_all, NULL);
+	struct __nf_ct_flush_report fr = {
+		.pid 	= pid,
+		.report = report,
+	};
+	nf_ct_iterate_cleanup(net, kill_all, &fr);
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_flush);
 

@@ -1005,7 +1022,7 @@ static void nf_conntrack_cleanup_net(struct net *net)
 	nf_ct_event_cache_flush(net);
 	nf_conntrack_ecache_fini(net);
  i_see_dead_people:
-	nf_conntrack_flush(net);
+	nf_conntrack_flush(net, 0, 0);
 	if (atomic_read(&net->ct.count) != 0) {
 		schedule();
 		goto i_see_dead_people;
net/netfilter/nf_conntrack_ecache.c +11 -3
index a5f5e2e..dee4190 100644
@@ -35,9 +35,17 @@ static inline void
 __nf_ct_deliver_cached_events(struct nf_conntrack_ecache *ecache)
 {
 	if (nf_ct_is_confirmed(ecache->ct) && !nf_ct_is_dying(ecache->ct)
-	    && ecache->events)
-		atomic_notifier_call_chain(&nf_conntrack_chain, ecache->events,
-				    ecache->ct);
+	    && ecache->events) {
+		struct nf_ct_event item = {
+			.ct 	= ecache->ct,
+			.pid	= 0,
+			.report	= 0
+		};
+
+		atomic_notifier_call_chain(&nf_conntrack_chain,
+					   ecache->events,
+					   &item);
+	}
 
 	ecache->events = 0;
 	nf_ct_put(ecache->ct);
net/netfilter/nf_conntrack_expect.c +36 -7

72 lines not on this page · raw message

net/netfilter/nf_conntrack_netlink.c +73 -15

191 lines not on this page · raw message

Applied
#2 lore raw

Re: [PATCH 2/2] netfilter: ctnetlink: deliver events for conntracks changed from userspace

Quoted 10 lines · Pablo Neira Ayuso

As for now, the creation and update of conntracks via ctnetlink do not propagate an event to userspace. This can result in inconsistent situations if several userspace processes modify the connection tracking table by means of ctnetlink at the same time. Specifically, using the conntrack command line tool and conntrackd at the same time can trigger unconsistencies.

This patch also modifies the event cache infrastructure to pass the process PID and the ECHO flag to nfnetlink_send() to report back to userspace if the process that triggered the change needs so. Based on a suggestion from Patrick McHardy.

Applied, thanks.

Applied
#3 lore raw
In reply to Pablo Neira Ayuso
Quoted 9 lines · Pablo Neira Ayuso

This patch adds module loading for helpers via ctnetlink.

* Creation path: We support explicit and implicit helper assignation. For the explicit case, we try to load the module. If the module is correctly loaded and the helper is present, we return EAGAIN to re-start the creation. Otherwise, we return EOPNOTSUPP.
* Update path: release the spin lock, load the module and check. If it is present, then return EAGAIN to re-start the update.

Quoted 3 lines

This patch provides a refactorized function to lookup-and-set the connection tracking helper. The function removes the exported symbol __nf_ct_helper_find as it has not clients anymore.

Applied, thanks Pablo.