Skip to content

Commit

Permalink
netfilter: nf_tables: fix module unload race
Browse files Browse the repository at this point in the history
We must first remove the nfnetlink protocol handler when nf_tables module
is unloaded -- we don't want userspace to submit new change requests once
we've started to tear down nft state.

Furthermore, nfnetlink must not call any subsystem function after
call_batch returned -EAGAIN.

EAGAIN means the subsys mutex was dropped, so its unlikely but possible that
nf_tables subsystem was removed due to 'rmmod nf_tables' on another cpu.

Therefore, we must abort batch completely and not move on to next part of
the batch.

Last, we can't invoke ->abort unless we've checked that the subsystem is
still registered.

Change netns exit path of nf_tables to make sure any incompleted
transaction gets removed on exit.

Signed-off-by: Florian Westphal <[email protected]>
Signed-off-by: Pablo Neira Ayuso <[email protected]>
  • Loading branch information
Florian Westphal authored and ummakynes committed Jun 12, 2018
1 parent 215a31f commit 71ad00c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
12 changes: 9 additions & 3 deletions net/netfilter/nf_tables_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -6439,7 +6439,7 @@ static void nf_tables_abort_release(struct nft_trans *trans)
kfree(trans);
}

static int nf_tables_abort(struct net *net, struct sk_buff *skb)
static int __nf_tables_abort(struct net *net)
{
struct nft_trans *trans, *next;
struct nft_trans_elem *te;
Expand Down Expand Up @@ -6555,6 +6555,11 @@ static void nf_tables_cleanup(struct net *net)
nft_validate_state_update(net, NFT_VALIDATE_SKIP);
}

static int nf_tables_abort(struct net *net, struct sk_buff *skb)
{
return __nf_tables_abort(net);
}

static bool nf_tables_valid_genid(struct net *net, u32 genid)
{
return net->nft.base_seq == genid;
Expand Down Expand Up @@ -7149,9 +7154,10 @@ static int __net_init nf_tables_init_net(struct net *net)

static void __net_exit nf_tables_exit_net(struct net *net)
{
if (!list_empty(&net->nft.commit_list))
__nf_tables_abort(net);
__nft_release_tables(net);
WARN_ON_ONCE(!list_empty(&net->nft.tables));
WARN_ON_ONCE(!list_empty(&net->nft.commit_list));
}

static struct pernet_operations nf_tables_net_ops = {
Expand Down Expand Up @@ -7193,9 +7199,9 @@ static int __init nf_tables_module_init(void)

static void __exit nf_tables_module_exit(void)
{
unregister_pernet_subsys(&nf_tables_net_ops);
nfnetlink_subsys_unregister(&nf_tables_subsys);
unregister_netdevice_notifier(&nf_tables_flowtable_notifier);
unregister_pernet_subsys(&nf_tables_net_ops);
rcu_barrier();
nf_tables_core_module_exit();
kfree(info);
Expand Down
10 changes: 7 additions & 3 deletions net/netfilter/nfnetlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh,
*/
if (err == -EAGAIN) {
status |= NFNL_BATCH_REPLAY;
goto next;
goto done;
}
}
ack:
Expand All @@ -456,15 +456,19 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh,
if (err)
status |= NFNL_BATCH_FAILURE;
}
next:

msglen = NLMSG_ALIGN(nlh->nlmsg_len);
if (msglen > skb->len)
msglen = skb->len;
skb_pull(skb, msglen);
}
done:
if (status & NFNL_BATCH_REPLAY) {
ss->abort(net, oskb);
const struct nfnetlink_subsystem *ss2;

ss2 = nfnl_dereference_protected(subsys_id);
if (ss2 == ss)
ss->abort(net, oskb);
nfnl_err_reset(&err_list);
nfnl_unlock(subsys_id);
kfree_skb(skb);
Expand Down

0 comments on commit 71ad00c

Please sign in to comment.