Skip to content

Commit

Permalink
netfilter: nf_conntrack: make event callback registration per-netns
Browse files Browse the repository at this point in the history
This patch fixes an oops that can be triggered following this recipe:

0) make sure nf_conntrack_netlink and nf_conntrack_ipv4 are loaded.
1) container is started.
2) connect to it via lxc-console.
3) generate some traffic with the container to create some conntrack
   entries in its table.
4) stop the container: you hit one oops because the conntrack table
   cleanup tries to report the destroy event to user-space but the
   per-netns nfnetlink socket has already gone (as the nfnetlink
   socket is per-netns but event callback registration is global).

To fix this situation, we make the ctnl_notifier per-netns so the
callback is registered/unregistered if the container is
created/destroyed.

Alex Bligh and Alexey Dobriyan originally proposed one small patch to
check if the nfnetlink socket is gone in nfnetlink_has_listeners,
but this is a very visited path for events, thus, it may reduce
performance and it looks a bit hackish to check for the nfnetlink
socket only to workaround this situation. As a result, I decided
to follow the bigger path choice, which seems to look nicer to me.

Cc: Alexey Dobriyan <[email protected]>
Reported-by: Alex Bligh <[email protected]>
Signed-off-by: Pablo Neira Ayuso <[email protected]>
  • Loading branch information
ummakynes committed Nov 21, 2011
1 parent 5e2afba commit 70e9942
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 49 deletions.
19 changes: 10 additions & 9 deletions include/net/netfilter/nf_conntrack_ecache.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,18 @@ struct nf_ct_event_notifier {
int (*fcn)(unsigned int events, struct nf_ct_event *item);
};

extern struct nf_ct_event_notifier __rcu *nf_conntrack_event_cb;
extern int nf_conntrack_register_notifier(struct nf_ct_event_notifier *nb);
extern void nf_conntrack_unregister_notifier(struct nf_ct_event_notifier *nb);
extern int nf_conntrack_register_notifier(struct net *net, struct nf_ct_event_notifier *nb);
extern void nf_conntrack_unregister_notifier(struct net *net, struct nf_ct_event_notifier *nb);

extern void nf_ct_deliver_cached_events(struct nf_conn *ct);

static inline void
nf_conntrack_event_cache(enum ip_conntrack_events event, struct nf_conn *ct)
{
struct net *net = nf_ct_net(ct);
struct nf_conntrack_ecache *e;

if (nf_conntrack_event_cb == NULL)
if (net->ct.nf_conntrack_event_cb == NULL)
return;

e = nf_ct_ecache_find(ct);
Expand All @@ -95,11 +95,12 @@ nf_conntrack_eventmask_report(unsigned int eventmask,
int report)
{
int ret = 0;
struct net *net = nf_ct_net(ct);
struct nf_ct_event_notifier *notify;
struct nf_conntrack_ecache *e;

rcu_read_lock();
notify = rcu_dereference(nf_conntrack_event_cb);
notify = rcu_dereference(net->ct.nf_conntrack_event_cb);
if (notify == NULL)
goto out_unlock;

Expand Down Expand Up @@ -164,21 +165,21 @@ struct nf_exp_event_notifier {
int (*fcn)(unsigned int events, struct nf_exp_event *item);
};

extern struct nf_exp_event_notifier __rcu *nf_expect_event_cb;
extern int nf_ct_expect_register_notifier(struct nf_exp_event_notifier *nb);
extern void nf_ct_expect_unregister_notifier(struct nf_exp_event_notifier *nb);
extern int nf_ct_expect_register_notifier(struct net *net, struct nf_exp_event_notifier *nb);
extern void nf_ct_expect_unregister_notifier(struct net *net, struct nf_exp_event_notifier *nb);

static inline void
nf_ct_expect_event_report(enum ip_conntrack_expect_events event,
struct nf_conntrack_expect *exp,
u32 pid,
int report)
{
struct net *net = nf_ct_exp_net(exp);
struct nf_exp_event_notifier *notify;
struct nf_conntrack_ecache *e;

rcu_read_lock();
notify = rcu_dereference(nf_expect_event_cb);
notify = rcu_dereference(net->ct.nf_expect_event_cb);
if (notify == NULL)
goto out_unlock;

Expand Down
2 changes: 2 additions & 0 deletions include/net/netns/conntrack.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ struct netns_ct {
struct hlist_nulls_head unconfirmed;
struct hlist_nulls_head dying;
struct ip_conntrack_stat __percpu *stat;
struct nf_ct_event_notifier __rcu *nf_conntrack_event_cb;
struct nf_exp_event_notifier __rcu *nf_expect_event_cb;
int sysctl_events;
unsigned int sysctl_events_retry_timeout;
int sysctl_acct;
Expand Down
37 changes: 18 additions & 19 deletions net/netfilter/nf_conntrack_ecache.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,17 @@

static DEFINE_MUTEX(nf_ct_ecache_mutex);

struct nf_ct_event_notifier __rcu *nf_conntrack_event_cb __read_mostly;
EXPORT_SYMBOL_GPL(nf_conntrack_event_cb);

struct nf_exp_event_notifier __rcu *nf_expect_event_cb __read_mostly;
EXPORT_SYMBOL_GPL(nf_expect_event_cb);

/* deliver cached events and clear cache entry - must be called with locally
* disabled softirqs */
void nf_ct_deliver_cached_events(struct nf_conn *ct)
{
struct net *net = nf_ct_net(ct);
unsigned long events;
struct nf_ct_event_notifier *notify;
struct nf_conntrack_ecache *e;

rcu_read_lock();
notify = rcu_dereference(nf_conntrack_event_cb);
notify = rcu_dereference(net->ct.nf_conntrack_event_cb);
if (notify == NULL)
goto out_unlock;

Expand Down Expand Up @@ -83,19 +78,20 @@ void nf_ct_deliver_cached_events(struct nf_conn *ct)
}
EXPORT_SYMBOL_GPL(nf_ct_deliver_cached_events);

int nf_conntrack_register_notifier(struct nf_ct_event_notifier *new)
int nf_conntrack_register_notifier(struct net *net,
struct nf_ct_event_notifier *new)
{
int ret = 0;
struct nf_ct_event_notifier *notify;

mutex_lock(&nf_ct_ecache_mutex);
notify = rcu_dereference_protected(nf_conntrack_event_cb,
notify = rcu_dereference_protected(net->ct.nf_conntrack_event_cb,
lockdep_is_held(&nf_ct_ecache_mutex));
if (notify != NULL) {
ret = -EBUSY;
goto out_unlock;
}
RCU_INIT_POINTER(nf_conntrack_event_cb, new);
RCU_INIT_POINTER(net->ct.nf_conntrack_event_cb, new);
mutex_unlock(&nf_ct_ecache_mutex);
return ret;

Expand All @@ -105,32 +101,34 @@ int nf_conntrack_register_notifier(struct nf_ct_event_notifier *new)
}
EXPORT_SYMBOL_GPL(nf_conntrack_register_notifier);

void nf_conntrack_unregister_notifier(struct nf_ct_event_notifier *new)
void nf_conntrack_unregister_notifier(struct net *net,
struct nf_ct_event_notifier *new)
{
struct nf_ct_event_notifier *notify;

mutex_lock(&nf_ct_ecache_mutex);
notify = rcu_dereference_protected(nf_conntrack_event_cb,
notify = rcu_dereference_protected(net->ct.nf_conntrack_event_cb,
lockdep_is_held(&nf_ct_ecache_mutex));
BUG_ON(notify != new);
RCU_INIT_POINTER(nf_conntrack_event_cb, NULL);
RCU_INIT_POINTER(net->ct.nf_conntrack_event_cb, NULL);
mutex_unlock(&nf_ct_ecache_mutex);
}
EXPORT_SYMBOL_GPL(nf_conntrack_unregister_notifier);

int nf_ct_expect_register_notifier(struct nf_exp_event_notifier *new)
int nf_ct_expect_register_notifier(struct net *net,
struct nf_exp_event_notifier *new)
{
int ret = 0;
struct nf_exp_event_notifier *notify;

mutex_lock(&nf_ct_ecache_mutex);
notify = rcu_dereference_protected(nf_expect_event_cb,
notify = rcu_dereference_protected(net->ct.nf_expect_event_cb,
lockdep_is_held(&nf_ct_ecache_mutex));
if (notify != NULL) {
ret = -EBUSY;
goto out_unlock;
}
RCU_INIT_POINTER(nf_expect_event_cb, new);
RCU_INIT_POINTER(net->ct.nf_expect_event_cb, new);
mutex_unlock(&nf_ct_ecache_mutex);
return ret;

Expand All @@ -140,15 +138,16 @@ int nf_ct_expect_register_notifier(struct nf_exp_event_notifier *new)
}
EXPORT_SYMBOL_GPL(nf_ct_expect_register_notifier);

void nf_ct_expect_unregister_notifier(struct nf_exp_event_notifier *new)
void nf_ct_expect_unregister_notifier(struct net *net,
struct nf_exp_event_notifier *new)
{
struct nf_exp_event_notifier *notify;

mutex_lock(&nf_ct_ecache_mutex);
notify = rcu_dereference_protected(nf_expect_event_cb,
notify = rcu_dereference_protected(net->ct.nf_expect_event_cb,
lockdep_is_held(&nf_ct_ecache_mutex));
BUG_ON(notify != new);
RCU_INIT_POINTER(nf_expect_event_cb, NULL);
RCU_INIT_POINTER(net->ct.nf_expect_event_cb, NULL);
mutex_unlock(&nf_ct_ecache_mutex);
}
EXPORT_SYMBOL_GPL(nf_ct_expect_unregister_notifier);
Expand Down
73 changes: 52 additions & 21 deletions net/netfilter/nf_conntrack_netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* (C) 2001 by Jay Schulist <[email protected]>
* (C) 2002-2006 by Harald Welte <[email protected]>
* (C) 2003 by Patrick Mchardy <[email protected]>
* (C) 2005-2008 by Pablo Neira Ayuso <[email protected]>
* (C) 2005-2011 by Pablo Neira Ayuso <[email protected]>
*
* Initial connection tracking via netlink development funded and
* generally made possible by Network Robots, Inc. (www.networkrobots.com)
Expand Down Expand Up @@ -2163,6 +2163,54 @@ MODULE_ALIAS("ip_conntrack_netlink");
MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_CTNETLINK);
MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_CTNETLINK_EXP);

static int __net_init ctnetlink_net_init(struct net *net)
{
#ifdef CONFIG_NF_CONNTRACK_EVENTS
int ret;

ret = nf_conntrack_register_notifier(net, &ctnl_notifier);
if (ret < 0) {
pr_err("ctnetlink_init: cannot register notifier.\n");
goto err_out;
}

ret = nf_ct_expect_register_notifier(net, &ctnl_notifier_exp);
if (ret < 0) {
pr_err("ctnetlink_init: cannot expect register notifier.\n");
goto err_unreg_notifier;
}
#endif
return 0;

#ifdef CONFIG_NF_CONNTRACK_EVENTS
err_unreg_notifier:
nf_conntrack_unregister_notifier(net, &ctnl_notifier);
err_out:
return ret;
#endif
}

static void ctnetlink_net_exit(struct net *net)
{
#ifdef CONFIG_NF_CONNTRACK_EVENTS
nf_ct_expect_unregister_notifier(net, &ctnl_notifier_exp);
nf_conntrack_unregister_notifier(net, &ctnl_notifier);
#endif
}

static void __net_exit ctnetlink_net_exit_batch(struct list_head *net_exit_list)
{
struct net *net;

list_for_each_entry(net, net_exit_list, exit_list)
ctnetlink_net_exit(net);
}

static struct pernet_operations ctnetlink_net_ops = {
.init = ctnetlink_net_init,
.exit_batch = ctnetlink_net_exit_batch,
};

static int __init ctnetlink_init(void)
{
int ret;
Expand All @@ -2180,28 +2228,15 @@ static int __init ctnetlink_init(void)
goto err_unreg_subsys;
}

#ifdef CONFIG_NF_CONNTRACK_EVENTS
ret = nf_conntrack_register_notifier(&ctnl_notifier);
if (ret < 0) {
pr_err("ctnetlink_init: cannot register notifier.\n");
if (register_pernet_subsys(&ctnetlink_net_ops)) {
pr_err("ctnetlink_init: cannot register pernet operations\n");
goto err_unreg_exp_subsys;
}

ret = nf_ct_expect_register_notifier(&ctnl_notifier_exp);
if (ret < 0) {
pr_err("ctnetlink_init: cannot expect register notifier.\n");
goto err_unreg_notifier;
}
#endif

return 0;

#ifdef CONFIG_NF_CONNTRACK_EVENTS
err_unreg_notifier:
nf_conntrack_unregister_notifier(&ctnl_notifier);
err_unreg_exp_subsys:
nfnetlink_subsys_unregister(&ctnl_exp_subsys);
#endif
err_unreg_subsys:
nfnetlink_subsys_unregister(&ctnl_subsys);
err_out:
Expand All @@ -2213,11 +2248,7 @@ static void __exit ctnetlink_exit(void)
pr_info("ctnetlink: unregistering from nfnetlink.\n");

nf_ct_remove_userspace_expectations();
#ifdef CONFIG_NF_CONNTRACK_EVENTS
nf_ct_expect_unregister_notifier(&ctnl_notifier_exp);
nf_conntrack_unregister_notifier(&ctnl_notifier);
#endif

unregister_pernet_subsys(&ctnetlink_net_ops);
nfnetlink_subsys_unregister(&ctnl_exp_subsys);
nfnetlink_subsys_unregister(&ctnl_subsys);
}
Expand Down

0 comments on commit 70e9942

Please sign in to comment.