Commit e0535ce5 authored by Wolfgang Bumiller's avatar Wolfgang Bumiller Committed by David S. Miller

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: 1045ba77 ("net sched actions: Add support for user cookies")
Signed-off-by: default avatarWolfgang Bumiller <w.bumiller@proxmox.com>
Acked-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 43af3905
......@@ -529,20 +529,20 @@ int tcf_action_dump(struct sk_buff *skb, struct list_head *actions,
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);
}
......
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