Merge branch 'net_sched-fix-a-UAF-in-tcf_action_init'

Cong Wang says:

====================
net_sched: fix a UAF in tcf_action_init()

This patchset fixes a use-after-free triggered by syzbot. Please
find more details in each patch description.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
David S. Miller 2020-09-24 19:46:21 -07:00
commit 6d8899962a
22 changed files with 35 additions and 66 deletions

View File

@ -166,8 +166,6 @@ int tcf_idr_create_from_flags(struct tc_action_net *tn, u32 index,
struct nlattr *est, struct tc_action **a,
const struct tc_action_ops *ops, int bind,
u32 flags);
void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a);
void tcf_idr_cleanup(struct tc_action_net *tn, u32 index);
int tcf_idr_check_alloc(struct tc_action_net *tn, u32 *index,
struct tc_action **a, int bind);

View File

@ -307,6 +307,8 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
mutex_lock(&idrinfo->lock);
idr_for_each_entry_ul(idr, p, tmp, id) {
if (IS_ERR(p))
continue;
ret = tcf_idr_release_unsafe(p);
if (ret == ACT_P_DELETED) {
module_put(ops->owner);
@ -467,17 +469,6 @@ int tcf_idr_create_from_flags(struct tc_action_net *tn, u32 index,
}
EXPORT_SYMBOL(tcf_idr_create_from_flags);
void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a)
{
struct tcf_idrinfo *idrinfo = tn->idrinfo;
mutex_lock(&idrinfo->lock);
/* Replace ERR_PTR(-EBUSY) allocated by tcf_idr_check_alloc */
WARN_ON(!IS_ERR(idr_replace(&idrinfo->action_idr, a, a->tcfa_index)));
mutex_unlock(&idrinfo->lock);
}
EXPORT_SYMBOL(tcf_idr_insert);
/* Cleanup idr index that was allocated but not initialized. */
void tcf_idr_cleanup(struct tc_action_net *tn, u32 index)
@ -902,6 +893,26 @@ static const struct nla_policy tcf_action_policy[TCA_ACT_MAX + 1] = {
[TCA_ACT_HW_STATS] = NLA_POLICY_BITFIELD32(TCA_ACT_HW_STATS_ANY),
};
static void tcf_idr_insert_many(struct tc_action *actions[])
{
int i;
for (i = 0; i < TCA_ACT_MAX_PRIO; i++) {
struct tc_action *a = actions[i];
struct tcf_idrinfo *idrinfo;
if (!a)
continue;
idrinfo = a->idrinfo;
mutex_lock(&idrinfo->lock);
/* Replace ERR_PTR(-EBUSY) allocated by tcf_idr_check_alloc if
* it is just created, otherwise this is just a nop.
*/
idr_replace(&idrinfo->action_idr, a, a->tcfa_index);
mutex_unlock(&idrinfo->lock);
}
}
struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
struct nlattr *nla, struct nlattr *est,
char *name, int ovr, int bind,
@ -989,6 +1000,13 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
if (err < 0)
goto err_mod;
if (TC_ACT_EXT_CMP(a->tcfa_action, TC_ACT_GOTO_CHAIN) &&
!rcu_access_pointer(a->goto_chain)) {
tcf_action_destroy_1(a, bind);
NL_SET_ERR_MSG(extack, "can't use goto chain with NULL chain");
return ERR_PTR(-EINVAL);
}
if (!name && tb[TCA_ACT_COOKIE])
tcf_set_action_cookie(&a->act_cookie, cookie);
@ -1002,13 +1020,6 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
if (err != ACT_P_CREATED)
module_put(a_o->owner);
if (TC_ACT_EXT_CMP(a->tcfa_action, TC_ACT_GOTO_CHAIN) &&
!rcu_access_pointer(a->goto_chain)) {
tcf_action_destroy_1(a, bind);
NL_SET_ERR_MSG(extack, "can't use goto chain with NULL chain");
return ERR_PTR(-EINVAL);
}
return a;
err_mod:
@ -1051,6 +1062,11 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
actions[i - 1] = act;
}
/* We have to commit them all together, because if any error happened in
* between, we could not handle the failure gracefully.
*/
tcf_idr_insert_many(actions);
*attr_size = tcf_action_full_attrs_size(sz);
return i - 1;

View File

@ -365,9 +365,7 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
if (goto_ch)
tcf_chain_put_by_act(goto_ch);
if (res == ACT_P_CREATED) {
tcf_idr_insert(tn, *act);
} else {
if (res != ACT_P_CREATED) {
/* make sure the program being replaced is no longer executing */
synchronize_rcu();
tcf_bpf_cfg_cleanup(&old);

View File

@ -139,7 +139,6 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
ci->net = net;
ci->zone = parm->zone;
tcf_idr_insert(tn, *a);
ret = ACT_P_CREATED;
} else if (ret > 0) {
ci = to_connmark(*a);

View File

@ -110,9 +110,6 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
if (params_new)
kfree_rcu(params_new, rcu);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
put_chain:
if (goto_ch)

View File

@ -1297,8 +1297,6 @@ static int tcf_ct_init(struct net *net, struct nlattr *nla,
tcf_chain_put_by_act(goto_ch);
if (params)
call_rcu(&params->rcu, tcf_ct_params_free);
if (res == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return res;

View File

@ -269,9 +269,6 @@ static int tcf_ctinfo_init(struct net *net, struct nlattr *nla,
if (cp_new)
kfree_rcu(cp_new, rcu);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
put_chain:

View File

@ -140,8 +140,6 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
if (goto_ch)
tcf_chain_put_by_act(goto_ch);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
release_idr:
tcf_idr_release(*a, bind);

View File

@ -437,9 +437,6 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
if (goto_ch)
tcf_chain_put_by_act(goto_ch);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
chain_put:

View File

@ -627,9 +627,6 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
if (p)
kfree_rcu(p, rcu);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
metadata_parse_err:
if (goto_ch)

View File

@ -189,8 +189,6 @@ static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
ipt->tcfi_t = t;
ipt->tcfi_hook = hook;
spin_unlock_bh(&ipt->tcf_lock);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
err3:

View File

@ -194,8 +194,6 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
spin_lock(&mirred_list_lock);
list_add(&m->tcfm_list, &mirred_list);
spin_unlock(&mirred_list_lock);
tcf_idr_insert(tn, *a);
}
return ret;

View File

@ -273,8 +273,6 @@ static int tcf_mpls_init(struct net *net, struct nlattr *nla,
if (p)
kfree_rcu(p, rcu);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
put_chain:
if (goto_ch)

View File

@ -93,9 +93,6 @@ static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
if (goto_ch)
tcf_chain_put_by_act(goto_ch);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
release_idr:
tcf_idr_release(*a, bind);

View File

@ -238,8 +238,6 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
spin_unlock_bh(&p->tcf_lock);
if (goto_ch)
tcf_chain_put_by_act(goto_ch);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
put_chain:

View File

@ -201,8 +201,6 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
if (new)
kfree_rcu(new, rcu);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
failure:

View File

@ -116,8 +116,6 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
if (goto_ch)
tcf_chain_put_by_act(goto_ch);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
put_chain:
if (goto_ch)

View File

@ -157,8 +157,6 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
goto release_idr;
}
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
put_chain:
if (goto_ch)

View File

@ -225,8 +225,6 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
if (goto_ch)
tcf_chain_put_by_act(goto_ch);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
put_chain:
if (goto_ch)

View File

@ -190,8 +190,6 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
if (goto_ch)
tcf_chain_put_by_act(goto_ch);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
put_chain:
if (goto_ch)

View File

@ -537,9 +537,6 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
if (goto_ch)
tcf_chain_put_by_act(goto_ch);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
put_chain:

View File

@ -229,8 +229,6 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
if (p)
kfree_rcu(p, rcu);
if (ret == ACT_P_CREATED)
tcf_idr_insert(tn, *a);
return ret;
put_chain:
if (goto_ch)