[Zesty][PATCH v2 0/2] net sched actions: allocate act cookie early
Fabian Grünbichler
f.gruenbichler at proxmox.com
Mon Apr 24 14:29:26 UTC 2017
that should have been [PATCH 2/2] in the subject, sorry for the
confusion.
On Mon, Apr 24, 2017 at 04:26:39PM +0200, Fabian Grünbichler wrote:
> From: Wolfgang Bumiller <w.bumiller at proxmox.com>
>
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1682368
>
> 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 at proxmox.com>
> Acked-by: Jamal Hadi Salim <jhs at mojatatu.com>
> Signed-off-by: David S. Miller <davem at davemloft.net>
> (cherry picked from commit e0535ce58b92d7baf0b33284a6c4f8f0338f943e)
> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> ---
> net/sched/act_api.c | 55 +++++++++++++++++++++++++++++++----------------------
> 1 file changed, 32 insertions(+), 23 deletions(-)
>
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index e336f30..bdbc7a9 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -532,20 +532,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,
> @@ -554,6 +554,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;
> @@ -569,6 +570,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)
> @@ -607,20 +620,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
> @@ -635,6 +640,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);
> }
>
> --
> 2.1.4
>
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
More information about the kernel-team
mailing list