]>
Commit | Line | Data |
---|---|---|
7f0f6370 FG |
1 | From 3fe083491bf6c688d34c6e300f14d775a5b8a443 Mon Sep 17 00:00:00 2001 |
2 | From: Wolfgang Bumiller <w.bumiller@proxmox.com> | |
3 | Date: Mon, 24 Apr 2017 16:26:00 +0200 | |
4 | Subject: [PATCH 2/2] net sched actions: allocate act cookie early | |
5 | MIME-Version: 1.0 | |
6 | Content-Type: text/plain; charset=UTF-8 | |
7 | Content-Transfer-Encoding: 8bit | |
8 | ||
9 | https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1682368 | |
10 | ||
11 | Policing filters do not use the TCA_ACT_* enum and the tb[] | |
12 | nlattr array in tcf_action_init_1() doesn't get filled for | |
13 | them so we should not try to look for a TCA_ACT_COOKIE | |
14 | attribute in the then uninitialized array. | |
15 | The error handling in cookie allocation then calls | |
16 | tcf_hash_release() leading to invalid memory access later | |
17 | on. | |
18 | Additionally, if cookie allocation fails after an already | |
19 | existing non-policing filter has successfully been changed, | |
20 | tcf_action_release() should not be called, also we would | |
21 | have to roll back the changes in the error handling, so | |
22 | instead we now allocate the cookie early and assign it on | |
23 | success at the end. | |
24 | ||
25 | CVE-2017-7979 | |
26 | Fixes: 1045ba77a596 ("net sched actions: Add support for user cookies") | |
27 | Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com> | |
28 | Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> | |
29 | Signed-off-by: David S. Miller <davem@davemloft.net> | |
30 | (cherry picked from commit e0535ce58b92d7baf0b33284a6c4f8f0338f943e) | |
31 | Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> | |
32 | Acked-by: Kamal Mostafa <kamal@canonical.com> | |
33 | Acked-by: Seth Forshee <seth.forshee@canonical.com> | |
34 | Signed-off-by: Stefan Bader <stefan.bader@canonical.com> | |
35 | ||
36 | Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com> | |
37 | --- | |
38 | net/sched/act_api.c | 55 +++++++++++++++++++++++++++++++---------------------- | |
39 | 1 file changed, 32 insertions(+), 23 deletions(-) | |
40 | ||
41 | diff --git a/net/sched/act_api.c b/net/sched/act_api.c | |
42 | index e336f30..bdbc7a9 100644 | |
43 | --- a/net/sched/act_api.c | |
44 | +++ b/net/sched/act_api.c | |
45 | @@ -532,20 +532,20 @@ int tcf_action_dump(struct sk_buff *skb, struct list_head *actions, | |
46 | return err; | |
47 | } | |
48 | ||
49 | -static int nla_memdup_cookie(struct tc_action *a, struct nlattr **tb) | |
50 | +static struct tc_cookie *nla_memdup_cookie(struct nlattr **tb) | |
51 | { | |
52 | - a->act_cookie = kzalloc(sizeof(*a->act_cookie), GFP_KERNEL); | |
53 | - if (!a->act_cookie) | |
54 | - return -ENOMEM; | |
55 | + struct tc_cookie *c = kzalloc(sizeof(*c), GFP_KERNEL); | |
56 | + if (!c) | |
57 | + return NULL; | |
58 | ||
59 | - a->act_cookie->data = nla_memdup(tb[TCA_ACT_COOKIE], GFP_KERNEL); | |
60 | - if (!a->act_cookie->data) { | |
61 | - kfree(a->act_cookie); | |
62 | - return -ENOMEM; | |
63 | + c->data = nla_memdup(tb[TCA_ACT_COOKIE], GFP_KERNEL); | |
64 | + if (!c->data) { | |
65 | + kfree(c); | |
66 | + return NULL; | |
67 | } | |
68 | - a->act_cookie->len = nla_len(tb[TCA_ACT_COOKIE]); | |
69 | + c->len = nla_len(tb[TCA_ACT_COOKIE]); | |
70 | ||
71 | - return 0; | |
72 | + return c; | |
73 | } | |
74 | ||
75 | struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla, | |
76 | @@ -554,6 +554,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla, | |
77 | { | |
78 | struct tc_action *a; | |
79 | struct tc_action_ops *a_o; | |
80 | + struct tc_cookie *cookie = NULL; | |
81 | char act_name[IFNAMSIZ]; | |
82 | struct nlattr *tb[TCA_ACT_MAX + 1]; | |
83 | struct nlattr *kind; | |
84 | @@ -569,6 +570,18 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla, | |
85 | goto err_out; | |
86 | if (nla_strlcpy(act_name, kind, IFNAMSIZ) >= IFNAMSIZ) | |
87 | goto err_out; | |
88 | + if (tb[TCA_ACT_COOKIE]) { | |
89 | + int cklen = nla_len(tb[TCA_ACT_COOKIE]); | |
90 | + | |
91 | + if (cklen > TC_COOKIE_MAX_SIZE) | |
92 | + goto err_out; | |
93 | + | |
94 | + cookie = nla_memdup_cookie(tb); | |
95 | + if (!cookie) { | |
96 | + err = -ENOMEM; | |
97 | + goto err_out; | |
98 | + } | |
99 | + } | |
100 | } else { | |
101 | err = -EINVAL; | |
102 | if (strlcpy(act_name, name, IFNAMSIZ) >= IFNAMSIZ) | |
103 | @@ -607,20 +620,12 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla, | |
104 | if (err < 0) | |
105 | goto err_mod; | |
106 | ||
107 | - if (tb[TCA_ACT_COOKIE]) { | |
108 | - int cklen = nla_len(tb[TCA_ACT_COOKIE]); | |
109 | - | |
110 | - if (cklen > TC_COOKIE_MAX_SIZE) { | |
111 | - err = -EINVAL; | |
112 | - tcf_hash_release(a, bind); | |
113 | - goto err_mod; | |
114 | - } | |
115 | - | |
116 | - if (nla_memdup_cookie(a, tb) < 0) { | |
117 | - err = -ENOMEM; | |
118 | - tcf_hash_release(a, bind); | |
119 | - goto err_mod; | |
120 | + if (name == NULL && tb[TCA_ACT_COOKIE]) { | |
121 | + if (a->act_cookie) { | |
122 | + kfree(a->act_cookie->data); | |
123 | + kfree(a->act_cookie); | |
124 | } | |
125 | + a->act_cookie = cookie; | |
126 | } | |
127 | ||
128 | /* module count goes up only when brand new policy is created | |
129 | @@ -635,6 +640,10 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla, | |
130 | err_mod: | |
131 | module_put(a_o->owner); | |
132 | err_out: | |
133 | + if (cookie) { | |
134 | + kfree(cookie->data); | |
135 | + kfree(cookie); | |
136 | + } | |
137 | return ERR_PTR(err); | |
138 | } | |
139 | ||
140 | -- | |
141 | 2.1.4 | |
142 |