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: d9adf22a291883 ("netfilter: nf_tables: use call_rcu in netlink dumps")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 28241e82fd150c5aff18c8079118e7f2769ca09a..4b51599360344be440e9a5c1ca8d5a9a93487b7c 100644 (file)
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2270,13 +2270,13 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net,
                                    u32 flags, int family,
                                    const struct nft_table *table,
                                    const struct nft_chain *chain,
-                                   const struct nft_rule *rule)
+                                   const struct nft_rule *rule,
+                                   const struct nft_rule *prule)
 {
        struct nlmsghdr *nlh;
        struct nfgenmsg *nfmsg;
        const struct nft_expr *expr, *next;
        struct nlattr *list;
-       const struct nft_rule *prule;
        u16 type = nfnl_msg_type(NFNL_SUBSYS_NFTABLES, event);
 
        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,
                         NFTA_RULE_PAD))
                goto nla_put_failure;
 
-       if ((event != NFT_MSG_DELRULE) && (rule->list.prev != &chain->rules)) {
-               prule = list_prev_entry(rule, list);
+       if (event != NFT_MSG_DELRULE && prule) {
                if (nla_put_be64(skb, NFTA_RULE_POSITION,
                                 cpu_to_be64(prule->handle),
                                 NFTA_RULE_PAD))
@@ -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,
                                       event, 0, ctx->family, ctx->table,
-                                      ctx->chain, rule);
+                                      ctx->chain, rule, NULL);
        if (err < 0) {
                kfree_skb(skb);
                goto err;
@@ -2369,12 +2368,13 @@ static int __nf_tables_dump_rules(struct sk_buff *skb,
                                  const struct nft_chain *chain)
 {
        struct net *net = sock_net(skb->sk);
+       const struct nft_rule *rule, *prule;
        unsigned int s_idx = cb->args[0];
-       const struct nft_rule *rule;
 
+       prule = NULL;
        list_for_each_entry_rcu(rule, &chain->rules, list) {
                if (!nft_is_active(net, rule))
-                       goto cont;
+                       goto cont_skip;
                if (*idx < s_idx)
                        goto cont;
                if (*idx > s_idx) {
@@ -2386,11 +2386,13 @@ static int __nf_tables_dump_rules(struct sk_buff *skb,
                                        NFT_MSG_NEWRULE,
                                        NLM_F_MULTI | NLM_F_APPEND,
                                        table->family,
-                                       table, chain, rule) < 0)
+                                       table, chain, rule, prule) < 0)
                        return 1;
 
                nl_dump_check_consistent(cb, nlmsg_hdr(skb));
 cont:
+               prule = rule;
+cont_skip:
                (*idx)++;
        }
        return 0;
@@ -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,
                                       nlh->nlmsg_seq, NFT_MSG_NEWRULE, 0,
-                                      family, table, chain, rule);
+                                      family, table, chain, rule, NULL);
        if (err < 0)
                goto err;