Commit 2c82c7e7 authored by Florian Westphal's avatar Florian Westphal Committed by Pablo Neira Ayuso

netfilter: nf_tables: fix oops during rule dump

We can oops in nf_tables_fill_rule_info().

Its not possible to fetch previous element in rcu-protected lists
when deletions are not prevented somehow: list_del_rcu poisons
the ->prev pointer value.

Before rcu-conversion this was safe as dump operations did hold
nfnetlink mutex.

Pass previous rule as argument, obtained by keeping a pointer to
the previous rule during traversal.

Fixes: d9adf22a ("netfilter: nf_tables: use call_rcu in netlink dumps")
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent ee8a2b95
...@@ -2270,13 +2270,13 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net, ...@@ -2270,13 +2270,13 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net,
u32 flags, int family, u32 flags, int family,
const struct nft_table *table, const struct nft_table *table,
const struct nft_chain *chain, const struct nft_chain *chain,
const struct nft_rule *rule) const struct nft_rule *rule,
const struct nft_rule *prule)
{ {
struct nlmsghdr *nlh; struct nlmsghdr *nlh;
struct nfgenmsg *nfmsg; struct nfgenmsg *nfmsg;
const struct nft_expr *expr, *next; const struct nft_expr *expr, *next;
struct nlattr *list; struct nlattr *list;
const struct nft_rule *prule;
u16 type = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event); u16 type = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
nlh = nlmsg_put(skb, portid, seq, type, sizeof(struct nfgenmsg), flags); nlh = nlmsg_put(skb, portid, seq, type, sizeof(struct nfgenmsg), flags);
...@@ -2296,8 +2296,7 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net, ...@@ -2296,8 +2296,7 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net,
NFTA_RULE_PAD)) NFTA_RULE_PAD))
goto nla_put_failure; goto nla_put_failure;
if ((event != NFT_MSG_DELRULE) && (rule->list.prev != &chain->rules)) { if (event != NFT_MSG_DELRULE && prule) {
prule = list_prev_entry(rule, list);
if (nla_put_be64(skb, NFTA_RULE_POSITION, if (nla_put_be64(skb, NFTA_RULE_POSITION,
cpu_to_be64(prule->handle), cpu_to_be64(prule->handle),
NFTA_RULE_PAD)) NFTA_RULE_PAD))
...@@ -2344,7 +2343,7 @@ static void nf_tables_rule_notify(const struct nft_ctx *ctx, ...@@ -2344,7 +2343,7 @@ static void nf_tables_rule_notify(const struct nft_ctx *ctx,
err = nf_tables_fill_rule_info(skb, ctx->net, ctx->portid, ctx->seq, err = nf_tables_fill_rule_info(skb, ctx->net, ctx->portid, ctx->seq,
event, 0, ctx->family, ctx->table, event, 0, ctx->family, ctx->table,
ctx->chain, rule); ctx->chain, rule, NULL);
if (err < 0) { if (err < 0) {
kfree_skb(skb); kfree_skb(skb);
goto err; goto err;
...@@ -2369,12 +2368,13 @@ static int __nf_tables_dump_rules(struct sk_buff *skb, ...@@ -2369,12 +2368,13 @@ static int __nf_tables_dump_rules(struct sk_buff *skb,
const struct nft_chain *chain) const struct nft_chain *chain)
{ {
struct net *net = sock_net(skb->sk); struct net *net = sock_net(skb->sk);
const struct nft_rule *rule, *prule;
unsigned int s_idx = cb->args[0]; unsigned int s_idx = cb->args[0];
const struct nft_rule *rule;
prule = NULL;
list_for_each_entry_rcu(rule, &chain->rules, list) { list_for_each_entry_rcu(rule, &chain->rules, list) {
if (!nft_is_active(net, rule)) if (!nft_is_active(net, rule))
goto cont; goto cont_skip;
if (*idx < s_idx) if (*idx < s_idx)
goto cont; goto cont;
if (*idx > s_idx) { if (*idx > s_idx) {
...@@ -2386,11 +2386,13 @@ static int __nf_tables_dump_rules(struct sk_buff *skb, ...@@ -2386,11 +2386,13 @@ static int __nf_tables_dump_rules(struct sk_buff *skb,
NFT_MSG_NEWRULE, NFT_MSG_NEWRULE,
NLM_F_MULTI | NLM_F_APPEND, NLM_F_MULTI | NLM_F_APPEND,
table->family, table->family,
table, chain, rule) < 0) table, chain, rule, prule) < 0)
return 1; return 1;
nl_dump_check_consistent(cb, nlmsg_hdr(skb)); nl_dump_check_consistent(cb, nlmsg_hdr(skb));
cont: cont:
prule = rule;
cont_skip:
(*idx)++; (*idx)++;
} }
return 0; return 0;
...@@ -2546,7 +2548,7 @@ static int nf_tables_getrule(struct net *net, struct sock *nlsk, ...@@ -2546,7 +2548,7 @@ static int nf_tables_getrule(struct net *net, struct sock *nlsk,
err = nf_tables_fill_rule_info(skb2, net, NETLINK_CB(skb).portid, err = nf_tables_fill_rule_info(skb2, net, NETLINK_CB(skb).portid,
nlh->nlmsg_seq, NFT_MSG_NEWRULE, 0, nlh->nlmsg_seq, NFT_MSG_NEWRULE, 0,
family, table, chain, rule); family, table, chain, rule, NULL);
if (err < 0) if (err < 0)
goto err; goto err;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment