Skip to content

Commit

Permalink
netfilter: nf_tables: safe RCU iteration on list when dumping
Browse files Browse the repository at this point in the history 
The dump operation through netlink is not protected by the nfnl_lock.
Thus, a reader process can be dumping any of the existing object
lists while another process can be updating the list content.

This patch resolves this situation by protecting all the object
lists with RCU in the netlink dump path which is the reader side.
The updater path is already protected via nfnl_lock, so use list
manipulation RCU-safe operations.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
  • Loading branch information
Pablo Neira Ayuso committed Jul 14, 2014
1 parent 63283dd commit e688a7f
Showing 1 changed file with 53 additions and 41 deletions.
94 changes: 53 additions & 41 deletions net/netfilter/nf_tables_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ int nft_register_afinfo(struct net *net, struct nft_af_info *afi)
{
INIT_LIST_HEAD(&afi->tables);
nfnl_lock(NFNL_SUBSYS_NFTABLES);
list_add_tail(&afi->list, &net->nft.af_info);
list_add_tail_rcu(&afi->list, &net->nft.af_info);
nfnl_unlock(NFNL_SUBSYS_NFTABLES);
return 0;
}
Expand All @@ -51,7 +51,7 @@ EXPORT_SYMBOL_GPL(nft_register_afinfo);
void nft_unregister_afinfo(struct nft_af_info *afi)
{
nfnl_lock(NFNL_SUBSYS_NFTABLES);
list_del(&afi->list);
list_del_rcu(&afi->list);
nfnl_unlock(NFNL_SUBSYS_NFTABLES);
}
EXPORT_SYMBOL_GPL(nft_unregister_afinfo);
Expand Down Expand Up @@ -277,11 +277,12 @@ static int nf_tables_dump_tables(struct sk_buff *skb,
struct net *net = sock_net(skb->sk);
int family = nfmsg->nfgen_family;

list_for_each_entry(afi, &net->nft.af_info, list) {
rcu_read_lock();
list_for_each_entry_rcu(afi, &net->nft.af_info, list) {
if (family != NFPROTO_UNSPEC && family != afi->family)
continue;

list_for_each_entry(table, &afi->tables, list) {
list_for_each_entry_rcu(table, &afi->tables, list) {
if (idx < s_idx)
goto cont;
if (idx > s_idx)
Expand All @@ -299,6 +300,7 @@ static int nf_tables_dump_tables(struct sk_buff *skb,
}
}
done:
rcu_read_unlock();
cb->args[0] = idx;
return skb->len;
}
Expand Down Expand Up @@ -517,7 +519,7 @@ static int nf_tables_newtable(struct sock *nlsk, struct sk_buff *skb,
module_put(afi->owner);
return err;
}
list_add_tail(&table->list, &afi->tables);
list_add_tail_rcu(&table->list, &afi->tables);
return 0;
}

Expand Down Expand Up @@ -549,7 +551,7 @@ static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
if (err < 0)
return err;

list_del(&table->list);
list_del_rcu(&table->list);
return 0;
}

Expand Down Expand Up @@ -764,12 +766,13 @@ static int nf_tables_dump_chains(struct sk_buff *skb,
struct net *net = sock_net(skb->sk);
int family = nfmsg->nfgen_family;

list_for_each_entry(afi, &net->nft.af_info, list) {
rcu_read_lock();
list_for_each_entry_rcu(afi, &net->nft.af_info, list) {
if (family != NFPROTO_UNSPEC && family != afi->family)
continue;

list_for_each_entry(table, &afi->tables, list) {
list_for_each_entry(chain, &table->chains, list) {
list_for_each_entry_rcu(table, &afi->tables, list) {
list_for_each_entry_rcu(chain, &table->chains, list) {
if (idx < s_idx)
goto cont;
if (idx > s_idx)
Expand All @@ -787,11 +790,11 @@ static int nf_tables_dump_chains(struct sk_buff *skb,
}
}
done:
rcu_read_unlock();
cb->args[0] = idx;
return skb->len;
}


static int nf_tables_getchain(struct sock *nlsk, struct sk_buff *skb,
const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
Expand Down Expand Up @@ -1133,7 +1136,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
goto err2;

table->use++;
list_add_tail(&chain->list, &table->chains);
list_add_tail_rcu(&chain->list, &table->chains);
return 0;
err2:
if (!(table->flags & NFT_TABLE_F_DORMANT) &&
Expand Down Expand Up @@ -1183,7 +1186,7 @@ static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
return err;

table->use--;
list_del(&chain->list);
list_del_rcu(&chain->list);
return 0;
}

Expand All @@ -1202,9 +1205,9 @@ int nft_register_expr(struct nft_expr_type *type)
{
nfnl_lock(NFNL_SUBSYS_NFTABLES);
if (type->family == NFPROTO_UNSPEC)
list_add_tail(&type->list, &nf_tables_expressions);
list_add_tail_rcu(&type->list, &nf_tables_expressions);
else
list_add(&type->list, &nf_tables_expressions);
list_add_rcu(&type->list, &nf_tables_expressions);
nfnl_unlock(NFNL_SUBSYS_NFTABLES);
return 0;
}
Expand All @@ -1219,7 +1222,7 @@ EXPORT_SYMBOL_GPL(nft_register_expr);
void nft_unregister_expr(struct nft_expr_type *type)
{
nfnl_lock(NFNL_SUBSYS_NFTABLES);
list_del(&type->list);
list_del_rcu(&type->list);
nfnl_unlock(NFNL_SUBSYS_NFTABLES);
}
EXPORT_SYMBOL_GPL(nft_unregister_expr);
Expand Down Expand Up @@ -1555,13 +1558,14 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
u8 genctr = ACCESS_ONCE(net->nft.genctr);
u8 gencursor = ACCESS_ONCE(net->nft.gencursor);

list_for_each_entry(afi, &net->nft.af_info, list) {
rcu_read_lock();
list_for_each_entry_rcu(afi, &net->nft.af_info, list) {
if (family != NFPROTO_UNSPEC && family != afi->family)
continue;

list_for_each_entry(table, &afi->tables, list) {
list_for_each_entry(chain, &table->chains, list) {
list_for_each_entry(rule, &chain->rules, list) {
list_for_each_entry_rcu(table, &afi->tables, list) {
list_for_each_entry_rcu(chain, &table->chains, list) {
list_for_each_entry_rcu(rule, &chain->rules, list) {
if (!nft_rule_is_active(net, rule))
goto cont;
if (idx < s_idx)
Expand All @@ -1582,6 +1586,8 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
}
}
done:
rcu_read_unlock();

/* Invalidate this dump, a transition to the new generation happened */
if (gencursor != net->nft.gencursor || genctr != net->nft.genctr)
return -EBUSY;
Expand Down Expand Up @@ -1935,7 +1941,7 @@ static LIST_HEAD(nf_tables_set_ops);
int nft_register_set(struct nft_set_ops *ops)
{
nfnl_lock(NFNL_SUBSYS_NFTABLES);
list_add_tail(&ops->list, &nf_tables_set_ops);
list_add_tail_rcu(&ops->list, &nf_tables_set_ops);
nfnl_unlock(NFNL_SUBSYS_NFTABLES);
return 0;
}
Expand All @@ -1944,7 +1950,7 @@ EXPORT_SYMBOL_GPL(nft_register_set);
void nft_unregister_set(struct nft_set_ops *ops)
{
nfnl_lock(NFNL_SUBSYS_NFTABLES);
list_del(&ops->list);
list_del_rcu(&ops->list);
nfnl_unlock(NFNL_SUBSYS_NFTABLES);
}
EXPORT_SYMBOL_GPL(nft_unregister_set);
Expand Down Expand Up @@ -2237,7 +2243,8 @@ static int nf_tables_dump_sets_table(struct nft_ctx *ctx, struct sk_buff *skb,
if (cb->args[1])
return skb->len;

list_for_each_entry(set, &ctx->table->sets, list) {
rcu_read_lock();
list_for_each_entry_rcu(set, &ctx->table->sets, list) {
if (idx < s_idx)
goto cont;
if (nf_tables_fill_set(skb, ctx, set, NFT_MSG_NEWSET,
Expand All @@ -2250,6 +2257,7 @@ static int nf_tables_dump_sets_table(struct nft_ctx *ctx, struct sk_buff *skb,
}
cb->args[1] = 1;
done:
rcu_read_unlock();
return skb->len;
}

Expand All @@ -2263,7 +2271,8 @@ static int nf_tables_dump_sets_family(struct nft_ctx *ctx, struct sk_buff *skb,
if (cb->args[1])
return skb->len;

list_for_each_entry(table, &ctx->afi->tables, list) {
rcu_read_lock();
list_for_each_entry_rcu(table, &ctx->afi->tables, list) {
if (cur_table) {
if (cur_table != table)
continue;
Expand All @@ -2272,7 +2281,7 @@ static int nf_tables_dump_sets_family(struct nft_ctx *ctx, struct sk_buff *skb,
}
ctx->table = table;
idx = 0;
list_for_each_entry(set, &ctx->table->sets, list) {
list_for_each_entry_rcu(set, &ctx->table->sets, list) {
if (idx < s_idx)
goto cont;
if (nf_tables_fill_set(skb, ctx, set, NFT_MSG_NEWSET,
Expand All @@ -2287,6 +2296,7 @@ static int nf_tables_dump_sets_family(struct nft_ctx *ctx, struct sk_buff *skb,
}
cb->args[1] = 1;
done:
rcu_read_unlock();
return skb->len;
}

Expand All @@ -2303,15 +2313,16 @@ static int nf_tables_dump_sets_all(struct nft_ctx *ctx, struct sk_buff *skb,
if (cb->args[1])
return skb->len;

list_for_each_entry(afi, &net->nft.af_info, list) {
rcu_read_lock();
list_for_each_entry_rcu(afi, &net->nft.af_info, list) {
if (cur_family) {
if (afi->family != cur_family)
continue;

cur_family = 0;
}

list_for_each_entry(table, &afi->tables, list) {
list_for_each_entry_rcu(table, &afi->tables, list) {
if (cur_table) {
if (cur_table != table)
continue;
Expand All @@ -2322,7 +2333,7 @@ static int nf_tables_dump_sets_all(struct nft_ctx *ctx, struct sk_buff *skb,
ctx->table = table;
ctx->afi = afi;
idx = 0;
list_for_each_entry(set, &ctx->table->sets, list) {
list_for_each_entry_rcu(set, &ctx->table->sets, list) {
if (idx < s_idx)
goto cont;
if (nf_tables_fill_set(skb, ctx, set,
Expand All @@ -2342,6 +2353,7 @@ static int nf_tables_dump_sets_all(struct nft_ctx *ctx, struct sk_buff *skb,
}
cb->args[1] = 1;
done:
rcu_read_unlock();
return skb->len;
}

Expand Down Expand Up @@ -2600,7 +2612,7 @@ static int nf_tables_newset(struct sock *nlsk, struct sk_buff *skb,
if (err < 0)
goto err2;

list_add_tail(&set->list, &table->sets);
list_add_tail_rcu(&set->list, &table->sets);
table->use++;
return 0;

Expand All @@ -2620,7 +2632,7 @@ static void nft_set_destroy(struct nft_set *set)

static void nf_tables_set_destroy(const struct nft_ctx *ctx, struct nft_set *set)
{
list_del(&set->list);
list_del_rcu(&set->list);
nf_tables_set_notify(ctx, set, NFT_MSG_DELSET, GFP_ATOMIC);
nft_set_destroy(set);
}
Expand Down Expand Up @@ -2655,7 +2667,7 @@ static int nf_tables_delset(struct sock *nlsk, struct sk_buff *skb,
if (err < 0)
return err;

list_del(&set->list);
list_del_rcu(&set->list);
ctx.table->use--;
return 0;
}
Expand Down Expand Up @@ -2707,14 +2719,14 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
}
bind:
binding->chain = ctx->chain;
list_add_tail(&binding->list, &set->bindings);
list_add_tail_rcu(&binding->list, &set->bindings);
return 0;
}

void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
struct nft_set_binding *binding)
{
list_del(&binding->list);
list_del_rcu(&binding->list);

if (list_empty(&set->bindings) && set->flags & NFT_SET_ANONYMOUS &&
!(set->flags & NFT_SET_INACTIVE))
Expand Down Expand Up @@ -3494,12 +3506,12 @@ static int nf_tables_abort(struct sk_buff *skb)
}
nft_trans_destroy(trans);
} else {
list_del(&trans->ctx.table->list);
list_del_rcu(&trans->ctx.table->list);
}
break;
case NFT_MSG_DELTABLE:
list_add_tail(&trans->ctx.table->list,
&trans->ctx.afi->tables);
list_add_tail_rcu(&trans->ctx.table->list,
&trans->ctx.afi->tables);
nft_trans_destroy(trans);
break;
case NFT_MSG_NEWCHAIN:
Expand All @@ -3510,7 +3522,7 @@ static int nf_tables_abort(struct sk_buff *skb)
nft_trans_destroy(trans);
} else {
trans->ctx.table->use--;
list_del(&trans->ctx.chain->list);
list_del_rcu(&trans->ctx.chain->list);
if (!(trans->ctx.table->flags & NFT_TABLE_F_DORMANT) &&
trans->ctx.chain->flags & NFT_BASE_CHAIN) {
nf_unregister_hooks(nft_base_chain(trans->ctx.chain)->ops,
Expand All @@ -3520,8 +3532,8 @@ static int nf_tables_abort(struct sk_buff *skb)
break;
case NFT_MSG_DELCHAIN:
trans->ctx.table->use++;
list_add_tail(&trans->ctx.chain->list,
&trans->ctx.table->chains);
list_add_tail_rcu(&trans->ctx.chain->list,
&trans->ctx.table->chains);
nft_trans_destroy(trans);
break;
case NFT_MSG_NEWRULE:
Expand All @@ -3535,12 +3547,12 @@ static int nf_tables_abort(struct sk_buff *skb)
break;
case NFT_MSG_NEWSET:
trans->ctx.table->use--;
list_del(&nft_trans_set(trans)->list);
list_del_rcu(&nft_trans_set(trans)->list);
break;
case NFT_MSG_DELSET:
trans->ctx.table->use++;
list_add_tail(&nft_trans_set(trans)->list,
&trans->ctx.table->sets);
list_add_tail_rcu(&nft_trans_set(trans)->list,
&trans->ctx.table->sets);
nft_trans_destroy(trans);
break;
case NFT_MSG_NEWSETELEM:
Expand Down

0 comments on commit e688a7f

Please sign in to comment.