Skip to content

Commit d66bedf

Browse files
brianwittegregkh
brianwitte
authored and
gregkh
committed
netfilter: nf_tables: revert commit_mutex usage in reset path
[ Upstream commit 7f261bb ] It causes circular lock dependency between commit_mutex, nfnl_subsys_ipset and nlk_cb_mutex when nft reset, ipset list, and iptables-nft with '-m set' rule run at the same time. Previous patches made it safe to run individual reset handlers concurrently so commit_mutex is no longer required to prevent this. Fixes: bd662c4 ("netfilter: nf_tables: Add locking for NFT_MSG_GETOBJ_RESET requests") Fixes: 3d483fa ("netfilter: nf_tables: Add locking for NFT_MSG_GETSETELEM_RESET requests") Fixes: 3cb03ed ("netfilter: nf_tables: Add locking for NFT_MSG_GETRULE_RESET requests") Link: https://lore.kernel.org/all/aUh_3mVRV8OrGsVo@strlen.de/ Reported-by: <syzbot+ff16b505ec9152e5f448@syzkaller.appspotmail.com> Closes: https://syzkaller.appspot.com/bug?extid=ff16b505ec9152e5f448 Signed-off-by: Brian Witte <brianwitte@mailfence.com> Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent b4a8b66 commit d66bedf

1 file changed

Lines changed: 42 additions & 206 deletions

File tree

‎net/netfilter/nf_tables_api.c‎

Lines changed: 42 additions & 206 deletions
Original file line numberDiff line numberDiff line change
@@ -3719,23 +3719,6 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
37193719
return skb->len;
37203720
}
37213721

3722-
static int nf_tables_dumpreset_rules(struct sk_buff *skb,
3723-
struct netlink_callback *cb)
3724-
{
3725-
struct nftables_pernet *nft_net = nft_pernet(sock_net(skb->sk));
3726-
int ret;
3727-
3728-
/* Mutex is held is to prevent that two concurrent dump-and-reset calls
3729-
* do not underrun counters and quotas. The commit_mutex is used for
3730-
* the lack a better lock, this is not transaction path.
3731-
*/
3732-
mutex_lock(&nft_net->commit_mutex);
3733-
ret = nf_tables_dump_rules(skb, cb);
3734-
mutex_unlock(&nft_net->commit_mutex);
3735-
3736-
return ret;
3737-
}
3738-
37393722
static int nf_tables_dump_rules_start(struct netlink_callback *cb)
37403723
{
37413724
struct nft_rule_dump_ctx *ctx = (void *)cb->ctx;
@@ -3755,16 +3738,10 @@ static int nf_tables_dump_rules_start(struct netlink_callback *cb)
37553738
return -ENOMEM;
37563739
}
37573740
}
3758-
return 0;
3759-
}
3760-
3761-
static int nf_tables_dumpreset_rules_start(struct netlink_callback *cb)
3762-
{
3763-
struct nft_rule_dump_ctx *ctx = (void *)cb->ctx;
3764-
3765-
ctx->reset = true;
3741+
if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) == NFT_MSG_GETRULE_RESET)
3742+
ctx->reset = true;
37663743

3767-
return nf_tables_dump_rules_start(cb);
3744+
return 0;
37683745
}
37693746

37703747
static int nf_tables_dump_rules_done(struct netlink_callback *cb)
@@ -3830,6 +3807,8 @@ static int nf_tables_getrule(struct sk_buff *skb, const struct nfnl_info *info,
38303807
u32 portid = NETLINK_CB(skb).portid;
38313808
struct net *net = info->net;
38323809
struct sk_buff *skb2;
3810+
bool reset = false;
3811+
char *buf;
38333812

38343813
if (info->nlh->nlmsg_flags & NLM_F_DUMP) {
38353814
struct netlink_dump_control c = {
@@ -3843,47 +3822,16 @@ static int nf_tables_getrule(struct sk_buff *skb, const struct nfnl_info *info,
38433822
return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c);
38443823
}
38453824

3846-
skb2 = nf_tables_getrule_single(portid, info, nla, false);
3847-
if (IS_ERR(skb2))
3848-
return PTR_ERR(skb2);
3849-
3850-
return nfnetlink_unicast(skb2, net, portid);
3851-
}
3852-
3853-
static int nf_tables_getrule_reset(struct sk_buff *skb,
3854-
const struct nfnl_info *info,
3855-
const struct nlattr * const nla[])
3856-
{
3857-
struct nftables_pernet *nft_net = nft_pernet(info->net);
3858-
u32 portid = NETLINK_CB(skb).portid;
3859-
struct net *net = info->net;
3860-
struct sk_buff *skb2;
3861-
char *buf;
3862-
3863-
if (info->nlh->nlmsg_flags & NLM_F_DUMP) {
3864-
struct netlink_dump_control c = {
3865-
.start= nf_tables_dumpreset_rules_start,
3866-
.dump = nf_tables_dumpreset_rules,
3867-
.done = nf_tables_dump_rules_done,
3868-
.module = THIS_MODULE,
3869-
.data = (void *)nla,
3870-
};
3871-
3872-
return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c);
3873-
}
3874-
3875-
if (!try_module_get(THIS_MODULE))
3876-
return -EINVAL;
3877-
rcu_read_unlock();
3878-
mutex_lock(&nft_net->commit_mutex);
3879-
skb2 = nf_tables_getrule_single(portid, info, nla, true);
3880-
mutex_unlock(&nft_net->commit_mutex);
3881-
rcu_read_lock();
3882-
module_put(THIS_MODULE);
3825+
if (NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_GETRULE_RESET)
3826+
reset = true;
38833827

3828+
skb2 = nf_tables_getrule_single(portid, info, nla, reset);
38843829
if (IS_ERR(skb2))
38853830
return PTR_ERR(skb2);
38863831

3832+
if (!reset)
3833+
return nfnetlink_unicast(skb2, net, portid);
3834+
38873835
buf = kasprintf(GFP_ATOMIC, "%.*s:%u",
38883836
nla_len(nla[NFTA_RULE_TABLE]),
38893837
(char *)nla_data(nla[NFTA_RULE_TABLE]),
@@ -6107,6 +6055,10 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb)
61076055
nla_nest_end(skb, nest);
61086056
nlmsg_end(skb, nlh);
61096057

6058+
if (dump_ctx->reset && args.iter.count > args.iter.skip)
6059+
audit_log_nft_set_reset(table, cb->seq,
6060+
args.iter.count - args.iter.skip);
6061+
61106062
rcu_read_unlock();
61116063

61126064
if (args.iter.err && args.iter.err != -EMSGSIZE)
@@ -6122,26 +6074,6 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb)
61226074
return -ENOSPC;
61236075
}
61246076

6125-
static int nf_tables_dumpreset_set(struct sk_buff *skb,
6126-
struct netlink_callback *cb)
6127-
{
6128-
struct nftables_pernet *nft_net = nft_pernet(sock_net(skb->sk));
6129-
struct nft_set_dump_ctx *dump_ctx = cb->data;
6130-
int ret, skip = cb->args[0];
6131-
6132-
mutex_lock(&nft_net->commit_mutex);
6133-
6134-
ret = nf_tables_dump_set(skb, cb);
6135-
6136-
if (cb->args[0] > skip)
6137-
audit_log_nft_set_reset(dump_ctx->ctx.table, cb->seq,
6138-
cb->args[0] - skip);
6139-
6140-
mutex_unlock(&nft_net->commit_mutex);
6141-
6142-
return ret;
6143-
}
6144-
61456077
static int nf_tables_dump_set_start(struct netlink_callback *cb)
61466078
{
61476079
struct nft_set_dump_ctx *dump_ctx = cb->data;
@@ -6385,8 +6317,13 @@ static int nf_tables_getsetelem(struct sk_buff *skb,
63856317
{
63866318
struct netlink_ext_ack *extack = info->extack;
63876319
struct nft_set_dump_ctx dump_ctx;
6320+
int rem, err = 0, nelems = 0;
6321+
struct net *net = info->net;
63886322
struct nlattr *attr;
6389-
int rem, err = 0;
6323+
bool reset = false;
6324+
6325+
if (NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_GETSETELEM_RESET)
6326+
reset = true;
63906327

63916328
if (info->nlh->nlmsg_flags & NLM_F_DUMP) {
63926329
struct netlink_dump_control c = {
@@ -6396,7 +6333,7 @@ static int nf_tables_getsetelem(struct sk_buff *skb,
63966333
.module = THIS_MODULE,
63976334
};
63986335

6399-
err = nft_set_dump_ctx_init(&dump_ctx, skb, info, nla, false);
6336+
err = nft_set_dump_ctx_init(&dump_ctx, skb, info, nla, reset);
64006337
if (err)
64016338
return err;
64026339

@@ -6407,75 +6344,21 @@ static int nf_tables_getsetelem(struct sk_buff *skb,
64076344
if (!nla[NFTA_SET_ELEM_LIST_ELEMENTS])
64086345
return -EINVAL;
64096346

6410-
err = nft_set_dump_ctx_init(&dump_ctx, skb, info, nla, false);
6347+
err = nft_set_dump_ctx_init(&dump_ctx, skb, info, nla, reset);
64116348
if (err)
64126349
return err;
64136350

64146351
nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) {
6415-
err = nft_get_set_elem(&dump_ctx.ctx, dump_ctx.set, attr, false);
6416-
if (err < 0) {
6417-
NL_SET_BAD_ATTR(extack, attr);
6418-
break;
6419-
}
6420-
}
6421-
6422-
return err;
6423-
}
6424-
6425-
static int nf_tables_getsetelem_reset(struct sk_buff *skb,
6426-
const struct nfnl_info *info,
6427-
const struct nlattr * const nla[])
6428-
{
6429-
struct nftables_pernet *nft_net = nft_pernet(info->net);
6430-
struct netlink_ext_ack *extack = info->extack;
6431-
struct nft_set_dump_ctx dump_ctx;
6432-
int rem, err = 0, nelems = 0;
6433-
struct nlattr *attr;
6434-
6435-
if (info->nlh->nlmsg_flags & NLM_F_DUMP) {
6436-
struct netlink_dump_control c = {
6437-
.start = nf_tables_dump_set_start,
6438-
.dump = nf_tables_dumpreset_set,
6439-
.done = nf_tables_dump_set_done,
6440-
.module = THIS_MODULE,
6441-
};
6442-
6443-
err = nft_set_dump_ctx_init(&dump_ctx, skb, info, nla, true);
6444-
if (err)
6445-
return err;
6446-
6447-
c.data = &dump_ctx;
6448-
return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c);
6449-
}
6450-
6451-
if (!nla[NFTA_SET_ELEM_LIST_ELEMENTS])
6452-
return -EINVAL;
6453-
6454-
if (!try_module_get(THIS_MODULE))
6455-
return -EINVAL;
6456-
rcu_read_unlock();
6457-
mutex_lock(&nft_net->commit_mutex);
6458-
rcu_read_lock();
6459-
6460-
err = nft_set_dump_ctx_init(&dump_ctx, skb, info, nla, true);
6461-
if (err)
6462-
goto out_unlock;
6463-
6464-
nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) {
6465-
err = nft_get_set_elem(&dump_ctx.ctx, dump_ctx.set, attr, true);
6352+
err = nft_get_set_elem(&dump_ctx.ctx, dump_ctx.set, attr, reset);
64666353
if (err < 0) {
64676354
NL_SET_BAD_ATTR(extack, attr);
64686355
break;
64696356
}
64706357
nelems++;
64716358
}
6472-
audit_log_nft_set_reset(dump_ctx.ctx.table, nft_base_seq(info->net), nelems);
6473-
6474-
out_unlock:
6475-
rcu_read_unlock();
6476-
mutex_unlock(&nft_net->commit_mutex);
6477-
rcu_read_lock();
6478-
module_put(THIS_MODULE);
6359+
if (reset)
6360+
audit_log_nft_set_reset(dump_ctx.ctx.table, nft_base_seq(net),
6361+
nelems);
64796362

64806363
return err;
64816364
}
@@ -8182,19 +8065,6 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
81828065
return skb->len;
81838066
}
81848067

8185-
static int nf_tables_dumpreset_obj(struct sk_buff *skb,
8186-
struct netlink_callback *cb)
8187-
{
8188-
struct nftables_pernet *nft_net = nft_pernet(sock_net(skb->sk));
8189-
int ret;
8190-
8191-
mutex_lock(&nft_net->commit_mutex);
8192-
ret = nf_tables_dump_obj(skb, cb);
8193-
mutex_unlock(&nft_net->commit_mutex);
8194-
8195-
return ret;
8196-
}
8197-
81988068
static int nf_tables_dump_obj_start(struct netlink_callback *cb)
81998069
{
82008070
struct nft_obj_dump_ctx *ctx = (void *)cb->ctx;
@@ -8211,16 +8081,10 @@ static int nf_tables_dump_obj_start(struct netlink_callback *cb)
82118081
if (nla[NFTA_OBJ_TYPE])
82128082
ctx->type = ntohl(nla_get_be32(nla[NFTA_OBJ_TYPE]));
82138083

8214-
return 0;
8215-
}
8216-
8217-
static int nf_tables_dumpreset_obj_start(struct netlink_callback *cb)
8218-
{
8219-
struct nft_obj_dump_ctx *ctx = (void *)cb->ctx;
8220-
8221-
ctx->reset = true;
8084+
if (NFNL_MSG_TYPE(cb->nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET)
8085+
ctx->reset = true;
82228086

8223-
return nf_tables_dump_obj_start(cb);
8087+
return 0;
82248088
}
82258089

82268090
static int nf_tables_dump_obj_done(struct netlink_callback *cb)
@@ -8282,42 +8146,16 @@ nf_tables_getobj_single(u32 portid, const struct nfnl_info *info,
82828146
static int nf_tables_getobj(struct sk_buff *skb, const struct nfnl_info *info,
82838147
const struct nlattr * const nla[])
82848148
{
8285-
u32 portid = NETLINK_CB(skb).portid;
8286-
struct sk_buff *skb2;
8287-
8288-
if (info->nlh->nlmsg_flags & NLM_F_DUMP) {
8289-
struct netlink_dump_control c = {
8290-
.start = nf_tables_dump_obj_start,
8291-
.dump = nf_tables_dump_obj,
8292-
.done = nf_tables_dump_obj_done,
8293-
.module = THIS_MODULE,
8294-
.data = (void *)nla,
8295-
};
8296-
8297-
return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c);
8298-
}
8299-
8300-
skb2 = nf_tables_getobj_single(portid, info, nla, false);
8301-
if (IS_ERR(skb2))
8302-
return PTR_ERR(skb2);
8303-
8304-
return nfnetlink_unicast(skb2, info->net, portid);
8305-
}
8306-
8307-
static int nf_tables_getobj_reset(struct sk_buff *skb,
8308-
const struct nfnl_info *info,
8309-
const struct nlattr * const nla[])
8310-
{
8311-
struct nftables_pernet *nft_net = nft_pernet(info->net);
83128149
u32 portid = NETLINK_CB(skb).portid;
83138150
struct net *net = info->net;
83148151
struct sk_buff *skb2;
8152+
bool reset = false;
83158153
char *buf;
83168154

83178155
if (info->nlh->nlmsg_flags & NLM_F_DUMP) {
83188156
struct netlink_dump_control c = {
8319-
.start = nf_tables_dumpreset_obj_start,
8320-
.dump = nf_tables_dumpreset_obj,
8157+
.start = nf_tables_dump_obj_start,
8158+
.dump = nf_tables_dump_obj,
83218159
.done = nf_tables_dump_obj_done,
83228160
.module = THIS_MODULE,
83238161
.data = (void *)nla,
@@ -8326,18 +8164,16 @@ static int nf_tables_getobj_reset(struct sk_buff *skb,
83268164
return nft_netlink_dump_start_rcu(info->sk, skb, info->nlh, &c);
83278165
}
83288166

8329-
if (!try_module_get(THIS_MODULE))
8330-
return -EINVAL;
8331-
rcu_read_unlock();
8332-
mutex_lock(&nft_net->commit_mutex);
8333-
skb2 = nf_tables_getobj_single(portid, info, nla, true);
8334-
mutex_unlock(&nft_net->commit_mutex);
8335-
rcu_read_lock();
8336-
module_put(THIS_MODULE);
8167+
if (NFNL_MSG_TYPE(info->nlh->nlmsg_type) == NFT_MSG_GETOBJ_RESET)
8168+
reset = true;
83378169

8170+
skb2 = nf_tables_getobj_single(portid, info, nla, reset);
83388171
if (IS_ERR(skb2))
83398172
return PTR_ERR(skb2);
83408173

8174+
if (!reset)
8175+
return nfnetlink_unicast(skb2, net, NETLINK_CB(skb).portid);
8176+
83418177
buf = kasprintf(GFP_ATOMIC, "%.*s:%u",
83428178
nla_len(nla[NFTA_OBJ_TABLE]),
83438179
(char *)nla_data(nla[NFTA_OBJ_TABLE]),
@@ -9540,7 +9376,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
95409376
.policy = nft_rule_policy,
95419377
},
95429378
[NFT_MSG_GETRULE_RESET] = {
9543-
.call = nf_tables_getrule_reset,
9379+
.call = nf_tables_getrule,
95449380
.type = NFNL_CB_RCU,
95459381
.attr_count = NFTA_RULE_MAX,
95469382
.policy = nft_rule_policy,
@@ -9594,7 +9430,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
95949430
.policy = nft_set_elem_list_policy,
95959431
},
95969432
[NFT_MSG_GETSETELEM_RESET] = {
9597-
.call = nf_tables_getsetelem_reset,
9433+
.call = nf_tables_getsetelem,
95989434
.type = NFNL_CB_RCU,
95999435
.attr_count = NFTA_SET_ELEM_LIST_MAX,
96009436
.policy = nft_set_elem_list_policy,
@@ -9640,7 +9476,7 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
96409476
.policy = nft_obj_policy,
96419477
},
96429478
[NFT_MSG_GETOBJ_RESET] = {
9643-
.call = nf_tables_getobj_reset,
9479+
.call = nf_tables_getobj,
96449480
.type = NFNL_CB_RCU,
96459481
.attr_count = NFTA_OBJ_MAX,
96469482
.policy = nft_obj_policy,

0 commit comments

Comments
 (0)