diff options
author | Cong Wang <xiyou.wangcong@gmail.com> | 2014-05-19 15:15:49 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2014-05-21 16:47:13 -0400 |
commit | bf63ac73b3e132e6bf0c8798aba7b277c3316e19 (patch) | |
tree | bfbe1ee0ca1dbc6e422d0b4d7c06acd0f893e0ba /net | |
parent | 78ff4be45a4c51d8fb21ad92e4fabb467c6c3eeb (diff) |
net_sched: fix an oops in tcindex filter
Kelly reported the following crash:
IP: [<ffffffff817a993d>] tcf_action_exec+0x46/0x90
PGD 3009067 PUD 300c067 PMD 11ff30067 PTE 800000011634b060
Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
CPU: 1 PID: 639 Comm: dhclient Not tainted 3.15.0-rc4+ #342
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
task: ffff8801169ecd00 ti: ffff8800d21b8000 task.ti: ffff8800d21b8000
RIP: 0010:[<ffffffff817a993d>] [<ffffffff817a993d>] tcf_action_exec+0x46/0x90
RSP: 0018:ffff8800d21b9b90 EFLAGS: 00010283
RAX: 00000000ffffffff RBX: ffff88011634b8e8 RCX: ffff8800cf7133d8
RDX: ffff88011634b900 RSI: ffff8800cf7133e0 RDI: ffff8800d210f840
RBP: ffff8800d21b9bb0 R08: ffffffff8287bf60 R09: 0000000000000001
R10: ffff8800d2b22b24 R11: 0000000000000001 R12: ffff8800d210f840
R13: ffff8800d21b9c50 R14: ffff8800cf7133e0 R15: ffff8800cad433d8
FS: 00007f49723e1840(0000) GS:ffff88011a800000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffff88011634b8f0 CR3: 00000000ce469000 CR4: 00000000000006e0
Stack:
ffff8800d2170188 ffff8800d210f840 ffff8800d2171b90 0000000000000000
ffff8800d21b9be8 ffffffff817c55bb ffff8800d21b9c50 ffff8800d2171b90
ffff8800d210f840 ffff8800d21b0300 ffff8800d21b9c50 ffff8800d21b9c18
Call Trace:
[<ffffffff817c55bb>] tcindex_classify+0x88/0x9b
[<ffffffff817a7f7d>] tc_classify_compat+0x3e/0x7b
[<ffffffff817a7fdf>] tc_classify+0x25/0x9f
[<ffffffff817b0e68>] htb_enqueue+0x55/0x27a
[<ffffffff817b6c2e>] dsmark_enqueue+0x165/0x1a4
[<ffffffff81775642>] __dev_queue_xmit+0x35e/0x536
[<ffffffff8177582a>] dev_queue_xmit+0x10/0x12
[<ffffffff818f8ecd>] packet_sendmsg+0xb26/0xb9a
[<ffffffff810b1507>] ? __lock_acquire+0x3ae/0xdf3
[<ffffffff8175cf08>] __sock_sendmsg_nosec+0x25/0x27
[<ffffffff8175d916>] sock_aio_write+0xd0/0xe7
[<ffffffff8117d6b8>] do_sync_write+0x59/0x78
[<ffffffff8117d84d>] vfs_write+0xb5/0x10a
[<ffffffff8117d96a>] SyS_write+0x49/0x7f
[<ffffffff8198e212>] system_call_fastpath+0x16/0x1b
This is because we memcpy struct tcindex_filter_result which contains
struct tcf_exts, obviously struct list_head can not be simply copied.
This is a regression introduced by commit 33be627159913b094bb578
(net_sched: act: use standard struct list_head).
It's not very easy to fix it as the code is a mess:
if (old_r)
memcpy(&cr, r, sizeof(cr));
else {
memset(&cr, 0, sizeof(cr));
tcf_exts_init(&cr.exts, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
}
...
tcf_exts_change(tp, &cr.exts, &e);
...
memcpy(r, &cr, sizeof(cr));
the above code should equal to:
tcindex_filter_result_init(&cr);
if (old_r)
cr.res = r->res;
...
if (old_r)
tcf_exts_change(tp, &r->exts, &e);
else
tcf_exts_change(tp, &cr.exts, &e);
...
r->res = cr.res;
after this change, since there is no need to copy struct tcf_exts.
And it also fixes other places zero'ing struct's contains struct tcf_exts.
Fixes: commit 33be627159913b0 (net_sched: act: use standard struct list_head)
Reported-by: Kelly Anderson <kelly@xilka.com>
Tested-by: Kelly Anderson <kelly@xilka.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net')
-rw-r--r-- | net/sched/cls_tcindex.c | 30 |
1 files changed, 20 insertions, 10 deletions
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c index eed8404443d8..f435a88d899a 100644 --- a/net/sched/cls_tcindex.c +++ b/net/sched/cls_tcindex.c | |||
@@ -188,6 +188,12 @@ static const struct nla_policy tcindex_policy[TCA_TCINDEX_MAX + 1] = { | |||
188 | [TCA_TCINDEX_CLASSID] = { .type = NLA_U32 }, | 188 | [TCA_TCINDEX_CLASSID] = { .type = NLA_U32 }, |
189 | }; | 189 | }; |
190 | 190 | ||
191 | static void tcindex_filter_result_init(struct tcindex_filter_result *r) | ||
192 | { | ||
193 | memset(r, 0, sizeof(*r)); | ||
194 | tcf_exts_init(&r->exts, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE); | ||
195 | } | ||
196 | |||
191 | static int | 197 | static int |
192 | tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, | 198 | tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, |
193 | u32 handle, struct tcindex_data *p, | 199 | u32 handle, struct tcindex_data *p, |
@@ -207,15 +213,11 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, | |||
207 | return err; | 213 | return err; |
208 | 214 | ||
209 | memcpy(&cp, p, sizeof(cp)); | 215 | memcpy(&cp, p, sizeof(cp)); |
210 | memset(&new_filter_result, 0, sizeof(new_filter_result)); | 216 | tcindex_filter_result_init(&new_filter_result); |
211 | tcf_exts_init(&new_filter_result.exts, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE); | ||
212 | 217 | ||
218 | tcindex_filter_result_init(&cr); | ||
213 | if (old_r) | 219 | if (old_r) |
214 | memcpy(&cr, r, sizeof(cr)); | 220 | cr.res = r->res; |
215 | else { | ||
216 | memset(&cr, 0, sizeof(cr)); | ||
217 | tcf_exts_init(&cr.exts, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE); | ||
218 | } | ||
219 | 221 | ||
220 | if (tb[TCA_TCINDEX_HASH]) | 222 | if (tb[TCA_TCINDEX_HASH]) |
221 | cp.hash = nla_get_u32(tb[TCA_TCINDEX_HASH]); | 223 | cp.hash = nla_get_u32(tb[TCA_TCINDEX_HASH]); |
@@ -267,9 +269,14 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, | |||
267 | err = -ENOMEM; | 269 | err = -ENOMEM; |
268 | if (!cp.perfect && !cp.h) { | 270 | if (!cp.perfect && !cp.h) { |
269 | if (valid_perfect_hash(&cp)) { | 271 | if (valid_perfect_hash(&cp)) { |
272 | int i; | ||
273 | |||
270 | cp.perfect = kcalloc(cp.hash, sizeof(*r), GFP_KERNEL); | 274 | cp.perfect = kcalloc(cp.hash, sizeof(*r), GFP_KERNEL); |
271 | if (!cp.perfect) | 275 | if (!cp.perfect) |
272 | goto errout; | 276 | goto errout; |
277 | for (i = 0; i < cp.hash; i++) | ||
278 | tcf_exts_init(&cp.perfect[i].exts, TCA_TCINDEX_ACT, | ||
279 | TCA_TCINDEX_POLICE); | ||
273 | balloc = 1; | 280 | balloc = 1; |
274 | } else { | 281 | } else { |
275 | cp.h = kcalloc(cp.hash, sizeof(f), GFP_KERNEL); | 282 | cp.h = kcalloc(cp.hash, sizeof(f), GFP_KERNEL); |
@@ -295,14 +302,17 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, | |||
295 | tcf_bind_filter(tp, &cr.res, base); | 302 | tcf_bind_filter(tp, &cr.res, base); |
296 | } | 303 | } |
297 | 304 | ||
298 | tcf_exts_change(tp, &cr.exts, &e); | 305 | if (old_r) |
306 | tcf_exts_change(tp, &r->exts, &e); | ||
307 | else | ||
308 | tcf_exts_change(tp, &cr.exts, &e); | ||
299 | 309 | ||
300 | tcf_tree_lock(tp); | 310 | tcf_tree_lock(tp); |
301 | if (old_r && old_r != r) | 311 | if (old_r && old_r != r) |
302 | memset(old_r, 0, sizeof(*old_r)); | 312 | tcindex_filter_result_init(old_r); |
303 | 313 | ||
304 | memcpy(p, &cp, sizeof(cp)); | 314 | memcpy(p, &cp, sizeof(cp)); |
305 | memcpy(r, &cr, sizeof(cr)); | 315 | r->res = cr.res; |
306 | 316 | ||
307 | if (r == &new_filter_result) { | 317 | if (r == &new_filter_result) { |
308 | struct tcindex_filter **fp; | 318 | struct tcindex_filter **fp; |