netfilter: nf_tables: Add locking for NFT_MSG_GETSETELEM_RESET requests

Set expressions' dump callbacks are not concurrency-safe per-se with
reset bit set. If two CPUs reset the same element at the same time,
values may underrun at least with element-attached counters and quotas.

Prevent this by introducing dedicated callbacks for nfnetlink and the
asynchronous dump handling to serialize access.

Signed-off-by: Phil Sutter <phil@nwl.cc>
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 9b58593e7729bc71da1db36ef25ee0ebe22d7ded..ed6b6bcd66088f3ddffb176e075436e8e5cde19c 100644 (file)
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -5817,10 +5817,6 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb)
        nla_nest_end(skb, nest);
        nlmsg_end(skb, nlh);
 
-       if (dump_ctx->reset && args.iter.count > args.iter.skip)
-               audit_log_nft_set_reset(table, cb->seq,
-                                       args.iter.count - args.iter.skip);
-
        rcu_read_unlock();
 
        if (args.iter.err && args.iter.err != -EMSGSIZE)
@@ -5836,6 +5832,26 @@ nla_put_failure:
        return -ENOSPC;
 }
 
+static int nf_tables_dumpreset_set(struct sk_buff *skb,
+                                  struct netlink_callback *cb)
+{
+       struct nftables_pernet *nft_net = nft_pernet(sock_net(skb->sk));
+       struct nft_set_dump_ctx *dump_ctx = cb->data;
+       int ret, skip = cb->args[0];
+
+       mutex_lock(&nft_net->commit_mutex);
+
+       ret = nf_tables_dump_set(skb, cb);
+
+       if (cb->args[0] > skip)
+               audit_log_nft_set_reset(dump_ctx->ctx.table, cb->seq,
+                                       cb->args[0] - skip);
+
+       mutex_unlock(&nft_net->commit_mutex);
+
+       return ret;
+}
+
 static int nf_tables_dump_set_start(struct netlink_callback *cb)
 {
        struct nft_set_dump_ctx *dump_ctx = cb->data;
@@ -6079,13 +6095,8 @@ static int nf_tables_getsetelem(struct sk_buff *skb,
 {
        struct netlink_ext_ack *extack = info->extack;
        struct nft_set_dump_ctx dump_ctx;
-       int rem, err = 0, nelems = 0;
-       struct net *net = info->net;
        struct nlattr *attr;
-       bool reset = false;
-
-       if (NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_GETSETELEM_RESET)
-               reset = true;
+       int rem, err = 0;
 
        if (info->nlh->nlmsg_flags & NLM_F_DUMP) {
                struct netlink_dump_control c = {
@@ -6095,7 +6106,7 @@ static int nf_tables_getsetelem(struct sk_buff *skb,
                        .module = THIS_MODULE,
                };
 
-               err = nft_set_dump_ctx_init(&dump_ctx, skb, info, nla, reset);
+               err = nft_set_dump_ctx_init(&dump_ctx, skb, info, nla, false);
                if (err)
                        return err;
 
@@ -6106,22 +6117,75 @@ static int nf_tables_getsetelem(struct sk_buff *skb,
        if (!nla[NFTA_SET_ELEM_LIST_ELEMENTS])
                return -EINVAL;
 
-       err = nft_set_dump_ctx_init(&dump_ctx, skb, info, nla, reset);
+       err = nft_set_dump_ctx_init(&dump_ctx, skb, info, nla, false);
        if (err)
                return err;
 
        nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) {
-               err = nft_get_set_elem(&dump_ctx.ctx, dump_ctx.set, attr, reset);
+               err = nft_get_set_elem(&dump_ctx.ctx, dump_ctx.set, attr, false);
+               if (err < 0) {
+                       NL_SET_BAD_ATTR(extack, attr);
+                       break;
+               }
+       }
+
+       return err;
+}
+
+static int nf_tables_getsetelem_reset(struct sk_buff *skb,
+                                     const struct nfnl_info *info,
+                                     const struct nlattr * const nla[])
+{
+       struct nftables_pernet *nft_net = nft_pernet(info->net);
+       struct netlink_ext_ack *extack = info->extack;
+       struct nft_set_dump_ctx dump_ctx;
+       int rem, err = 0, nelems = 0;
+       struct nlattr *attr;
+
+       if (info->nlh->nlmsg_flags & NLM_F_DUMP) {
+               struct netlink_dump_control c = {
+                       .start = nf_tables_dump_set_start,
+                       .dump = nf_tables_dumpreset_set,
+                       .done = nf_tables_dump_set_done,
+                       .module = THIS_MODULE,
+               };
+
+               err = nft_set_dump_ctx_init(&dump_ctx, skb, info, nla, true);
+               if (err)
+                       return err;
+
+               c.data = &dump_ctx;
+               return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c);
+       }
+
+       if (!nla[NFTA_SET_ELEM_LIST_ELEMENTS])
+               return -EINVAL;
+
+       if (!try_module_get(THIS_MODULE))
+               return -EINVAL;
+       rcu_read_unlock();
+       mutex_lock(&nft_net->commit_mutex);
+       rcu_read_lock();
+
+       err = nft_set_dump_ctx_init(&dump_ctx, skb, info, nla, true);
+       if (err)
+               goto out_unlock;
+
+       nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) {
+               err = nft_get_set_elem(&dump_ctx.ctx, dump_ctx.set, attr, true);
                if (err < 0) {
                        NL_SET_BAD_ATTR(extack, attr);
                        break;
                }
                nelems++;
        }
+       audit_log_nft_set_reset(dump_ctx.ctx.table, nft_net->base_seq, nelems);
 
-       if (reset)
-               audit_log_nft_set_reset(dump_ctx.ctx.table, nft_pernet(net)->base_seq,
-                                       nelems);
+out_unlock:
+       rcu_read_unlock();
+       mutex_unlock(&nft_net->commit_mutex);
+       rcu_read_lock();
+       module_put(THIS_MODULE);
 
        return err;
 }
@@ -9095,7 +9159,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
                .policy         = nft_set_elem_list_policy,
        },
        [NFT_MSG_GETSETELEM_RESET] = {
-               .call           = nf_tables_getsetelem,
+               .call           = nf_tables_getsetelem_reset,
                .type           = NFNL_CB_RCU,
                .attr_count     = NFTA_SET_ELEM_LIST_MAX,
                .policy         = nft_set_elem_list_policy,