All of lore.kernel.org
 help / color / mirror / Atom feed
From: gfree.wind@vip.163.com
To: pablo@netfilter.org, kadlec@blackhole.kfki.hu, fw@strlen.de,
	netfilter-devel@vger.kernel.org
Cc: Gao Feng <gfree.wind@vip.163.com>
Subject: [PATCH nf v6 3/3] netfilter: nat_helper: Remove the expectations when its module is unloaded
Date: Fri,  5 May 2017 08:55:13 +0800	[thread overview]
Message-ID: <1493945713-67263-3-git-send-email-gfree.wind@vip.163.com> (raw)
In-Reply-To: <1493945713-67263-1-git-send-email-gfree.wind@vip.163.com>

From: Gao Feng <gfree.wind@vip.163.com>

Because the conntrack NAT module could be rmmod anytime, so we should
really leave things in clean state if such thing happens and make sure
we don't leave any packet running over code that will be gone after the
removal.

We only removed the expectations when unregister conntrack helper before.
Actually it is necessary too when remove the nat helper.

Now add one new struct module member "me" in the nf_ct_nat_helper to
represent which module it belongs to.

Signed-off-by: Gao Feng <gfree.wind@vip.163.com>
---
 v6: Rename the helper name of ftp, tftp.. to ftp-nat-follow-master, per Pablo
 v5: Register one nat_helper for every nat module, per Pablo
 v4: Cover the nat_module assignment in dataplane, per Pablo
 v3: Rename the nf_ct_helper_expectfn, func, and member, per Pablo
 v2: Use the module as the identifier when flush expect
 v1: initial version

 include/net/netfilter/nf_conntrack_helper.h |  1 +
 net/ipv4/netfilter/nf_nat_h323.c            |  3 ++
 net/netfilter/ipvs/ip_vs_nfct.c             |  1 +
 net/netfilter/nf_conntrack_helper.c         | 62 +++++++++++++++++++----------
 net/netfilter/nf_conntrack_pptp.c           |  1 +
 net/netfilter/nf_nat_amanda.c               |  1 +
 net/netfilter/nf_nat_core.c                 |  1 +
 net/netfilter/nf_nat_ftp.c                  |  1 +
 net/netfilter/nf_nat_irc.c                  |  1 +
 net/netfilter/nf_nat_sip.c                  |  1 +
 net/netfilter/nf_nat_tftp.c                 |  1 +
 11 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_helper.h b/include/net/netfilter/nf_conntrack_helper.h
index e8d31ca..d70c5be 100644
--- a/include/net/netfilter/nf_conntrack_helper.h
+++ b/include/net/netfilter/nf_conntrack_helper.h
@@ -114,6 +114,7 @@ int nf_conntrack_broadcast_help(struct sk_buff *skb, unsigned int protoff,
 struct nf_ct_nat_helper {
 	struct list_head head;
 	const char *name;
+	struct module *me;
 	void (*expectfn)(struct nf_conn *ct, struct nf_conntrack_expect *exp);
 };
 
diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c
index ce2095c..d7b40ac 100644
--- a/net/ipv4/netfilter/nf_nat_h323.c
+++ b/net/ipv4/netfilter/nf_nat_h323.c
@@ -28,16 +28,19 @@ static void ip_nat_callforwarding_expect(struct nf_conn *new,
 
 static struct nf_ct_nat_helper q931_nat = {
 	.name		= "Q.931",
+	.me		= THIS_MODULE,
 	.expectfn	= ip_nat_q931_expect,
 };
 
 static struct nf_ct_nat_helper callforwarding_nat = {
 	.name		= "callforwarding",
+	.me		= THIS_MODULE,
 	.expectfn	= ip_nat_callforwarding_expect,
 };
 
 static struct nf_ct_nat_helper follow_master_nat = {
 	.name		= "h323-nat-follow-master",
+	.me		= THIS_MODULE,
 	.expectfn	= nf_nat_follow_master,
 };
 
diff --git a/net/netfilter/ipvs/ip_vs_nfct.c b/net/netfilter/ipvs/ip_vs_nfct.c
index fc5af4f..d61c236 100644
--- a/net/netfilter/ipvs/ip_vs_nfct.c
+++ b/net/netfilter/ipvs/ip_vs_nfct.c
@@ -222,6 +222,7 @@ static void ip_vs_nfct_expect_callback(struct nf_conn *ct,
 
 static struct nf_ct_nat_helper ip_vs_nat = {
 	.name = "ip-vs-nat",
+	.me = THIS_MODULE,
 	.expectfn = ip_vs_nfct_expect_callback,
 };
 
diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index 1fd739c..c12de31 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -130,6 +130,37 @@ static unsigned int helper_hash(const struct nf_conntrack_tuple *tuple)
 	return NULL;
 }
 
+static void
+nf_ct_flush_expect(const struct module *me)
+{
+	struct nf_conntrack_expect *exp;
+	const struct hlist_node *next;
+	u32 i;
+
+	if (!me)
+		return;
+
+	/* Get rid of expectations */
+	spin_lock_bh(&nf_conntrack_expect_lock);
+	for (i = 0; i < nf_ct_expect_hsize; i++) {
+		hlist_for_each_entry_safe(exp, next,
+					  &nf_ct_expect_hash[i], hnode) {
+			struct nf_conn_help *master_help = nfct_help(exp->master);
+
+			if ((master_help->helper && master_help->helper->me == me) ||
+			    (exp->helper && exp->helper->me == me) ||
+			    (exp->nat_helper && exp->nat_helper->me == me)) {
+				/* This expect belongs to the dying module */
+				if (del_timer(&exp->timeout)) {
+					nf_ct_unlink_expect(exp);
+					nf_ct_expect_put(exp);
+				}
+			}
+		}
+	}
+	spin_unlock_bh(&nf_conntrack_expect_lock);
+}
+
 struct nf_conntrack_helper *
 __nf_conntrack_helper_find(const char *name, u16 l3num, u8 protonum)
 {
@@ -317,6 +348,12 @@ void nf_ct_nat_helper_unregister(struct nf_ct_nat_helper *n)
 	spin_lock_bh(&nf_conntrack_expect_lock);
 	list_del_rcu(&n->head);
 	spin_unlock_bh(&nf_conntrack_expect_lock);
+
+	/* Make sure no one is still using the module unless
+	 * its a connection in the hash.
+	 */
+	synchronize_rcu();
+	nf_ct_flush_expect(n->me);
 }
 EXPORT_SYMBOL_GPL(nf_ct_nat_helper_unregister);
 
@@ -444,8 +481,6 @@ static void __nf_conntrack_helper_unregister(struct nf_conntrack_helper *me,
 void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me)
 {
 	struct nf_conntrack_tuple_hash *h;
-	struct nf_conntrack_expect *exp;
-	const struct hlist_node *next;
 	const struct hlist_nulls_node *nn;
 	unsigned int last_hsize;
 	spinlock_t *lock;
@@ -457,28 +492,11 @@ void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me)
 	nf_ct_helper_count--;
 	mutex_unlock(&nf_ct_helper_mutex);
 
-	/* Make sure every nothing is still using the helper unless its a
-	 * connection in the hash.
+	/* Make sure no one is still using the module unless
+	 * its a connection in the hash.
 	 */
 	synchronize_rcu();
-
-	/* Get rid of expectations */
-	spin_lock_bh(&nf_conntrack_expect_lock);
-	for (i = 0; i < nf_ct_expect_hsize; i++) {
-		hlist_for_each_entry_safe(exp, next,
-					  &nf_ct_expect_hash[i], hnode) {
-			struct nf_conn_help *help = nfct_help(exp->master);
-			if ((rcu_dereference_protected(
-					help->helper,
-					lockdep_is_held(&nf_conntrack_expect_lock)
-					) == me || exp->helper == me) &&
-			    del_timer(&exp->timeout)) {
-				nf_ct_unlink_expect(exp);
-				nf_ct_expect_put(exp);
-			}
-		}
-	}
-	spin_unlock_bh(&nf_conntrack_expect_lock);
+	nf_ct_flush_expect(me->me);
 
 	rtnl_lock();
 	for_each_net(net)
diff --git a/net/netfilter/nf_conntrack_pptp.c b/net/netfilter/nf_conntrack_pptp.c
index 896b154..581ec82 100644
--- a/net/netfilter/nf_conntrack_pptp.c
+++ b/net/netfilter/nf_conntrack_pptp.c
@@ -104,6 +104,7 @@ static void pptp_expectfn(struct nf_conn *ct,
 
 static struct nf_ct_nat_helper pptp_nat = {
 	.name			= "pptp-nat",
+	.me			= THIS_MODULE,
 	.expectfn		= pptp_expectfn,
 };
 
diff --git a/net/netfilter/nf_nat_amanda.c b/net/netfilter/nf_nat_amanda.c
index 9b540a9..e761e24 100644
--- a/net/netfilter/nf_nat_amanda.c
+++ b/net/netfilter/nf_nat_amanda.c
@@ -26,6 +26,7 @@
 
 static struct nf_ct_nat_helper amanda_nat = {
 	.name = "amanda-nat-follow-master",
+	.me = THIS_MODULE,
 	.expectfn = nf_nat_follow_master,
 };
 
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 2897bd4..ae1789b 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -850,6 +850,7 @@ static void __net_exit nf_nat_net_exit(struct net *net)
 
 static struct nf_ct_nat_helper follow_master_nat = {
 	.name		= "nat-follow-master",
+	.me		= THIS_MODULE,
 	.expectfn	= nf_nat_follow_master,
 };
 
diff --git a/net/netfilter/nf_nat_ftp.c b/net/netfilter/nf_nat_ftp.c
index f1f4501..82cc5e2 100644
--- a/net/netfilter/nf_nat_ftp.c
+++ b/net/netfilter/nf_nat_ftp.c
@@ -26,6 +26,7 @@
 
 static struct nf_ct_nat_helper ftp_nat = {
 	.name = "ftp-nat-follow-master",
+	.me = THIS_MODULE,
 	.expectfn = nf_nat_follow_master,
 };
 
diff --git a/net/netfilter/nf_nat_irc.c b/net/netfilter/nf_nat_irc.c
index cf623d3..0ce6746 100644
--- a/net/netfilter/nf_nat_irc.c
+++ b/net/netfilter/nf_nat_irc.c
@@ -28,6 +28,7 @@
 
 static struct nf_ct_nat_helper irc_nat = {
 	.name = "irc-nat-follow-master",
+	.me = THIS_MODULE,
 	.expectfn = nf_nat_follow_master,
 };
 
diff --git a/net/netfilter/nf_nat_sip.c b/net/netfilter/nf_nat_sip.c
index cd9333c..d13aae6 100644
--- a/net/netfilter/nf_nat_sip.c
+++ b/net/netfilter/nf_nat_sip.c
@@ -33,6 +33,7 @@ static void nf_nat_sip_expected(struct nf_conn *ct,
 
 static struct nf_ct_nat_helper sip_nat = {
 	.name		= "sip",
+	.me		= THIS_MODULE,
 	.expectfn	= nf_nat_sip_expected,
 };
 
diff --git a/net/netfilter/nf_nat_tftp.c b/net/netfilter/nf_nat_tftp.c
index d279b41..4e60158 100644
--- a/net/netfilter/nf_nat_tftp.c
+++ b/net/netfilter/nf_nat_tftp.c
@@ -20,6 +20,7 @@
 
 static struct nf_ct_nat_helper tftp_nat = {
 	.name = "tftp-nat-follow-master",
+	.me = THIS_MODULE,
 	.expectfn = nf_nat_follow_master,
 };
 
-- 
1.9.1



  parent reply	other threads:[~2017-05-05  1:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-05  0:55 [PATCH nf v6 1/3] netfilter: helper: Rename struct nf_ct_helper_expectfn to nf_ct_nat_helper gfree.wind
2017-05-05  0:55 ` [PATCH nf v6 2/3] netfilter: nat_helper: Register one nf_ct_nat_helper each proto nat module gfree.wind
2017-05-15 17:20   ` Pablo Neira Ayuso
2017-05-05  0:55 ` gfree.wind [this message]
2017-05-05  1:59   ` [PATCH nf v6 3/3] netfilter: nat_helper: Remove the expectations when its module is unloaded Liping Zhang
2017-05-05  2:16     ` Gao Feng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1493945713-67263-3-git-send-email-gfree.wind@vip.163.com \
    --to=gfree.wind@vip.163.com \
    --cc=fw@strlen.de \
    --cc=kadlec@blackhole.kfki.hu \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.