[nf] netfilter: helper: Fix possible panic after nf_conntrack_helper_unregister

Message ID 1528863973-99514-1-git-send-email-gfree.wind@vip.163.com
State Accepted
Delegated to: Pablo Neira
Headers show
Series
  • [nf] netfilter: helper: Fix possible panic after nf_conntrack_helper_unregister
Related show

Commit Message

Gao Feng June 13, 2018, 4:26 a.m.
From: Gao Feng <gfree.wind@vip.163.com>

The helper module would be unloaded after nf_conntrack_helper_unregister,
so it may cause a possible panic caused by race.

nf_ct_iterate_destroy(unhelp, me) reset the helper of conntrack as NULL,
but maybe someone has gotten the helper pointer during this period. Then
it would panic, when it accesses the helper and the module was unloaded.

Take an example as following:
CPU0                                                   CPU1
ctnetlink_dump_helpinfo
helper = rcu_dereference(help->helper);
                                                       unhelp
                                                       set helper as NULL
                                                       unload helper module
helper->to_nlattr(skb, ct);

As above, the cpu0 tries to access the helper and its module is unloaded,
then the panic happens.

Signed-off-by: Gao Feng <gfree.wind@vip.163.com>
---
 net/netfilter/nf_conntrack_helper.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Pablo Neira Ayuso June 18, 2018, 12:15 p.m. | #1
On Wed, Jun 13, 2018 at 12:26:13PM +0800, gfree.wind@vip.163.com wrote:
> From: Gao Feng <gfree.wind@vip.163.com>
> 
> The helper module would be unloaded after nf_conntrack_helper_unregister,
> so it may cause a possible panic caused by race.
> 
> nf_ct_iterate_destroy(unhelp, me) reset the helper of conntrack as NULL,
> but maybe someone has gotten the helper pointer during this period. Then
> it would panic, when it accesses the helper and the module was unloaded.

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index 551a1ed..b5b655d 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -465,6 +465,11 @@  void nf_conntrack_helper_unregister(struct nf_conntrack_helper *me)
 
 	nf_ct_expect_iterate_destroy(expect_iter_me, NULL);
 	nf_ct_iterate_destroy(unhelp, me);
+
+	/* Maybe someone has gotten the helper already when unhelp above.
+	 * So need to wait it.
+	 */
+	synchronize_rcu();
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_helper_unregister);