Commit bd662c42 authored by Phil Sutter's avatar Phil Sutter Committed by Pablo Neira Ayuso

netfilter: nf_tables: Add locking for NFT_MSG_GETOBJ_RESET requests

Objects' dump callbacks are not concurrency-safe per-se with reset bit
set. If two CPUs perform a reset at the same time, at least counter and
quota objects suffer from value underrun.

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

Fixes: 43da04a5 ("netfilter: nf_tables: atomic dump and reset for stateful objects")
Signed-off-by: default avatarPhil Sutter <phil@nwl.cc>
Reviewed-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 69fc3e9e
...@@ -8020,6 +8020,19 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb) ...@@ -8020,6 +8020,19 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
return skb->len; return skb->len;
} }
static int nf_tables_dumpreset_obj(struct sk_buff *skb,
struct netlink_callback *cb)
{
struct nftables_pernet *nft_net = nft_pernet(sock_net(skb->sk));
int ret;
mutex_lock(&nft_net->commit_mutex);
ret = nf_tables_dump_obj(skb, cb);
mutex_unlock(&nft_net->commit_mutex);
return ret;
}
static int nf_tables_dump_obj_start(struct netlink_callback *cb) static int nf_tables_dump_obj_start(struct netlink_callback *cb)
{ {
struct nft_obj_dump_ctx *ctx = (void *)cb->ctx; struct nft_obj_dump_ctx *ctx = (void *)cb->ctx;
...@@ -8036,10 +8049,16 @@ static int nf_tables_dump_obj_start(struct netlink_callback *cb) ...@@ -8036,10 +8049,16 @@ static int nf_tables_dump_obj_start(struct netlink_callback *cb)
if (nla[NFTA_OBJ_TYPE]) if (nla[NFTA_OBJ_TYPE])
ctx->type = ntohl(nla_get_be32(nla[NFTA_OBJ_TYPE])); ctx->type = ntohl(nla_get_be32(nla[NFTA_OBJ_TYPE]));
if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET) return 0;
}
static int nf_tables_dumpreset_obj_start(struct netlink_callback *cb)
{
struct nft_obj_dump_ctx *ctx = (void *)cb->ctx;
ctx->reset = true; ctx->reset = true;
return 0; return nf_tables_dump_obj_start(cb);
} }
static int nf_tables_dump_obj_done(struct netlink_callback *cb) static int nf_tables_dump_obj_done(struct netlink_callback *cb)
...@@ -8100,18 +8119,43 @@ nf_tables_getobj_single(u32 portid, const struct nfnl_info *info, ...@@ -8100,18 +8119,43 @@ nf_tables_getobj_single(u32 portid, const struct nfnl_info *info,
static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info, static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info,
const struct nlattr * const nla[]) const struct nlattr * const nla[])
{
u32 portid = NETLINK_CB(skb).portid;
struct sk_buff *skb2;
if (info->nlh->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
.start = nf_tables_dump_obj_start,
.dump = nf_tables_dump_obj,
.done = nf_tables_dump_obj_done,
.module = THIS_MODULE,
.data = (void *)nla,
};
return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c);
}
skb2 = nf_tables_getobj_single(portid, info, nla, false);
if (IS_ERR(skb2))
return PTR_ERR(skb2);
return nfnetlink_unicast(skb2, info->net, portid);
}
static int nf_tables_getobj_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 nftables_pernet *nft_net = nft_pernet(info->net);
u32 portid = NETLINK_CB(skb).portid; u32 portid = NETLINK_CB(skb).portid;
struct net *net = info->net; struct net *net = info->net;
struct sk_buff *skb2; struct sk_buff *skb2;
bool reset = false;
char *buf; char *buf;
if (info->nlh->nlmsg_flags & NLM_F_DUMP) { if (info->nlh->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = { struct netlink_dump_control c = {
.start = nf_tables_dump_obj_start, .start = nf_tables_dumpreset_obj_start,
.dump = nf_tables_dump_obj, .dump = nf_tables_dumpreset_obj,
.done = nf_tables_dump_obj_done, .done = nf_tables_dump_obj_done,
.module = THIS_MODULE, .module = THIS_MODULE,
.data = (void *)nla, .data = (void *)nla,
...@@ -8120,16 +8164,18 @@ static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info, ...@@ -8120,16 +8164,18 @@ static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info,
return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c); return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c);
} }
if (NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET) if (!try_module_get(THIS_MODULE))
reset = true; return -EINVAL;
rcu_read_unlock();
mutex_lock(&nft_net->commit_mutex);
skb2 = nf_tables_getobj_single(portid, info, nla, true);
mutex_unlock(&nft_net->commit_mutex);
rcu_read_lock();
module_put(THIS_MODULE);
skb2 = nf_tables_getobj_single(portid, info, nla, reset);
if (IS_ERR(skb2)) if (IS_ERR(skb2))
return PTR_ERR(skb2); return PTR_ERR(skb2);
if (!reset)
return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid);
buf = kasprintf(GFP_ATOMIC, "%.*s:%u", buf = kasprintf(GFP_ATOMIC, "%.*s:%u",
nla_len(nla[NFTA_OBJ_TABLE]), nla_len(nla[NFTA_OBJ_TABLE]),
(char *)nla_data(nla[NFTA_OBJ_TABLE]), (char *)nla_data(nla[NFTA_OBJ_TABLE]),
...@@ -9421,7 +9467,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = { ...@@ -9421,7 +9467,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
.policy = nft_obj_policy, .policy = nft_obj_policy,
}, },
[NFT_MSG_GETOBJ_RESET] = { [NFT_MSG_GETOBJ_RESET] = {
.call = nf_tables_getobj, .call = nf_tables_getobj_reset,
.type = NFNL_CB_RCU, .type = NFNL_CB_RCU,
.attr_count = NFTA_OBJ_MAX, .attr_count = NFTA_OBJ_MAX,
.policy = nft_obj_policy, .policy = nft_obj_policy,
......
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