summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorWolfgang Bumiller <w.bumiller@proxmox.com>2017-04-20 14:08:26 +0200
committerDavid S. Miller <davem@davemloft.net>2017-04-20 16:32:07 -0400
commite0535ce58b92d7baf0b33284a6c4f8f0338f943e (patch)
tree09149bb2d06e45a11b06bed0ce7c4afb453490a8
parent43af390573ad5539b6087831a1197747a0e672ad (diff)
net sched actions: allocate act cookie early
Policing filters do not use the TCA_ACT_* enum and the tb[] nlattr array in tcf_action_init_1() doesn't get filled for them so we should not try to look for a TCA_ACT_COOKIE attribute in the then uninitialized array. The error handling in cookie allocation then calls tcf_hash_release() leading to invalid memory access later on. Additionally, if cookie allocation fails after an already existing non-policing filter has successfully been changed, tcf_action_release() should not be called, also we would have to roll back the changes in the error handling, so instead we now allocate the cookie early and assign it on success at the end. CVE-2017-7979 Fixes: 1045ba77a596 ("net sched actions: Add support for user cookies") Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r--net/sched/act_api.c55
1 files changed, 32 insertions, 23 deletions
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index b70aa57319ea..e05b924618a0 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -529,20 +529,20 @@ errout:
return err;
}
-static int nla_memdup_cookie(struct tc_action *a, struct nlattr **tb)
+static struct tc_cookie *nla_memdup_cookie(struct nlattr **tb)
{
- a->act_cookie = kzalloc(sizeof(*a->act_cookie), GFP_KERNEL);
- if (!a->act_cookie)
- return -ENOMEM;
+ struct tc_cookie *c = kzalloc(sizeof(*c), GFP_KERNEL);
+ if (!c)
+ return NULL;
- a->act_cookie->data = nla_memdup(tb[TCA_ACT_COOKIE], GFP_KERNEL);
- if (!a->act_cookie->data) {
- kfree(a->act_cookie);
- return -ENOMEM;
+ c->data = nla_memdup(tb[TCA_ACT_COOKIE], GFP_KERNEL);
+ if (!c->data) {
+ kfree(c);
+ return NULL;
}
- a->act_cookie->len = nla_len(tb[TCA_ACT_COOKIE]);
+ c->len = nla_len(tb[TCA_ACT_COOKIE]);
- return 0;
+ return c;
}
struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
@@ -551,6 +551,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
{
struct tc_action *a;
struct tc_action_ops *a_o;
+ struct tc_cookie *cookie = NULL;
char act_name[IFNAMSIZ];
struct nlattr *tb[TCA_ACT_MAX + 1];
struct nlattr *kind;
@@ -566,6 +567,18 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
goto err_out;
if (nla_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ)
goto err_out;
+ if (tb[TCA_ACT_COOKIE]) {
+ int cklen = nla_len(tb[TCA_ACT_COOKIE]);
+
+ if (cklen > TC_COOKIE_MAX_SIZE)
+ goto err_out;
+
+ cookie = nla_memdup_cookie(tb);
+ if (!cookie) {
+ err = -ENOMEM;
+ goto err_out;
+ }
+ }
} else {
err = -EINVAL;
if (strlcpy(act_name, name, IFNAMSIZ) >= IFNAMSIZ)
@@ -604,20 +617,12 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
if (err < 0)
goto err_mod;
- if (tb[TCA_ACT_COOKIE]) {
- int cklen = nla_len(tb[TCA_ACT_COOKIE]);
-
- if (cklen > TC_COOKIE_MAX_SIZE) {
- err = -EINVAL;
- tcf_hash_release(a, bind);
- goto err_mod;
- }
-
- if (nla_memdup_cookie(a, tb) < 0) {
- err = -ENOMEM;
- tcf_hash_release(a, bind);
- goto err_mod;
+ if (name == NULL && tb[TCA_ACT_COOKIE]) {
+ if (a->act_cookie) {
+ kfree(a->act_cookie->data);
+ kfree(a->act_cookie);
}
+ a->act_cookie = cookie;
}
/* module count goes up only when brand new policy is created
@@ -632,6 +637,10 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
err_mod:
module_put(a_o->owner);
err_out:
+ if (cookie) {
+ kfree(cookie->data);
+ kfree(cookie);
+ }
return ERR_PTR(err);
}