leandrof/linux
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

net_sched: improve and refactor tcf_action_put_many()

Browse Source 
tcf_action_put_many() is mostly called to clean up actions on
failure path, but tcf_action_put_many(&actions[acts_deleted]) is
used in the ugliest way: it passes a slice of the array and
uses an additional NULL at the end to avoid out-of-bound
access.

acts_deleted is completely unnecessary since we can teach
tcf_action_put_many() scan the whole array and checks against
NULL pointer. Which also means tcf_action_delete() should
set deleted action pointers to NULL to avoid double free.

Fixes: 90b73b77d0 ("net: sched: change action API to use array of pointers to actions")
Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Vlad Buslov <vladbu@mellanox.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
Cong Wang 2018-08-19 12:22:05 -07:00 committed by David S. Miller
parent b93c1b5ac8
commit edfaf94fa7
1 changed files with 15 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
net/sched/act_api.c
View File

@ -686,14 +686,18 @@ static int tcf_action_put(struct tc_action *p)
return __tcf_action_put(p, false); return __tcf_action_put(p, false);
} }
/* Put all actions in this array, skip those NULL's. */
static void tcf_action_put_many(struct tc_action *actions[]) static void tcf_action_put_many(struct tc_action *actions[])
{ {
int i; int i;
for (i = 0; i < TCA_ACT_MAX_PRIO && actions[i]; i++) { for (i = 0; i < TCA_ACT_MAX_PRIO; i++) {
struct tc_action *a = actions[i]; struct tc_action *a = actions[i];
const struct tc_action_ops *ops = a->ops; const struct tc_action_ops *ops;
if (!a)
continue;
ops = a->ops;
if (tcf_action_put(a)) if (tcf_action_put(a))
module_put(ops->owner); module_put(ops->owner);
} }
@ -1176,7 +1180,7 @@ err_out:
} }
static int tcf_action_delete(struct net *net, struct tc_action *actions[], static int tcf_action_delete(struct net *net, struct tc_action *actions[],
int *acts_deleted, struct netlink_ext_ack *extack) struct netlink_ext_ack *extack)
{ {
u32 act_index; u32 act_index;
int ret, i; int ret, i;
@ -1196,20 +1200,17 @@ static int tcf_action_delete(struct net *net, struct tc_action *actions[],
} else { } else {
/* now do the delete */ /* now do the delete */
ret = ops->delete(net, act_index); ret = ops->delete(net, act_index);
if (ret < 0) { if (ret < 0)
*acts_deleted = i + 1;
return ret; return ret;
}
} }
actions[i] = NULL;
} }
*acts_deleted = i;
return 0; return 0;
} }
static int static int
tcf_del_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[], tcf_del_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[],
int *acts_deleted, u32 portid, size_t attr_size, u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
struct netlink_ext_ack *extack)
{ {
int ret; int ret;
struct sk_buff *skb; struct sk_buff *skb;
@ -1227,7 +1228,7 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[],
} }
/* now do the delete */ /* now do the delete */
ret = tcf_action_delete(net, actions, acts_deleted, extack); ret = tcf_action_delete(net, actions, extack);
if (ret < 0) { if (ret < 0) {
NL_SET_ERR_MSG(extack, "Failed to delete TC action"); NL_SET_ERR_MSG(extack, "Failed to delete TC action");
kfree_skb(skb); kfree_skb(skb);
@ -1249,8 +1250,7 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1]; struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act; struct tc_action *act;
size_t attr_size = 0; size_t attr_size = 0;
struct tc_action *actions[TCA_ACT_MAX_PRIO + 1] = {}; struct tc_action *actions[TCA_ACT_MAX_PRIO] = {};
int acts_deleted = 0;
ret = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, extack); ret = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, extack);
if (ret < 0) if (ret < 0)
@ -1280,14 +1280,13 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
if (event == RTM_GETACTION) if (event == RTM_GETACTION)
ret = tcf_get_notify(net, portid, n, actions, event, extack); ret = tcf_get_notify(net, portid, n, actions, event, extack);
else { /* delete */ else { /* delete */
ret = tcf_del_notify(net, n, actions, &acts_deleted, portid, ret = tcf_del_notify(net, n, actions, portid, attr_size, extack);
attr_size, extack);
if (ret) if (ret)
goto err; goto err;
return ret; return 0;
} }
err: err:
tcf_action_put_many(&actions[acts_deleted]); tcf_action_put_many(actions);
return ret; return ret;
} }