lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 30 Sep 2014 18:18:49 -0700 From: John Fastabend <john.fastabend@...il.com> To: Cong Wang <xiyou.wangcong@...il.com> CC: netdev@...r.kernel.org, davem@...emloft.net, John Fastabend <john.r.fastabend@...el.com> Subject: Re: [Patch net-next] net_sched: avoid calling tcf_unbind_filter() in call_rcu callback On 09/30/2014 05:53 PM, John Fastabend wrote: > On 09/30/2014 04:07 PM, Cong Wang wrote: >> This fixes the following crash: >> >> [ 63.976822] general protection fault: 0000 [#1] PREEMPT SMP >> DEBUG_PAGEALLOC >> [ 63.980094] CPU: 1 PID: 15 Comm: ksoftirqd/1 Not tainted >> 3.17.0-rc6+ #648 >> [ 63.980094] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 >> [ 63.980094] task: ffff880117dea690 ti: ffff880117dfc000 task.ti: >> ffff880117dfc000 >> [ 63.980094] RIP: 0010:[<ffffffff817e6d07>] [<ffffffff817e6d07>] >> u32_destroy_key+0x27/0x6d >> [ 63.980094] RSP: 0018:ffff880117dffcc0 EFLAGS: 00010202 >> [ 63.980094] RAX: ffff880117dea690 RBX: ffff8800d02e0820 RCX: >> 0000000000000000 >> [ 63.980094] RDX: 0000000000000001 RSI: 0000000000000002 RDI: >> 6b6b6b6b6b6b6b6b >> [ 63.980094] RBP: ffff880117dffcd0 R08: 0000000000000000 R09: >> 0000000000000000 >> [ 63.980094] R10: 00006c0900006ba8 R11: 00006ba100006b9d R12: >> 0000000000000001 >> [ 63.980094] R13: ffff8800d02e0898 R14: ffffffff817e6d4d R15: >> ffff880117387a30 >> [ 63.980094] FS: 0000000000000000(0000) GS:ffff88011a800000(0000) >> knlGS:0000000000000000 >> [ 63.980094] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b >> [ 63.980094] CR2: 00007f07e6732fed CR3: 000000011665b000 CR4: >> 00000000000006e0 >> [ 63.980094] Stack: >> [ 63.980094] ffff88011a9cd300 ffffffff82051ac0 ffff880117dffce0 >> ffffffff817e6d68 >> [ 63.980094] ffff880117dffd70 ffffffff810cb4c7 ffffffff810cb3cd >> ffff880117dfffd8 >> [ 63.980094] ffff880117dea690 ffff880117dea690 ffff880117dfffd8 >> 000000000000000a >> [ 63.980094] Call Trace: >> [ 63.980094] [<ffffffff817e6d68>] u32_delete_key_freepf_rcu+0x1b/0x1d >> [ 63.980094] [<ffffffff810cb4c7>] rcu_process_callbacks+0x3bb/0x691 >> [ 63.980094] [<ffffffff810cb3cd>] ? rcu_process_callbacks+0x2c1/0x691 >> [ 63.980094] [<ffffffff817e6d4d>] ? u32_destroy_key+0x6d/0x6d >> [ 63.980094] [<ffffffff810780a4>] __do_softirq+0x142/0x323 >> [ 63.980094] [<ffffffff810782a8>] run_ksoftirqd+0x23/0x53 >> [ 63.980094] [<ffffffff81092126>] smpboot_thread_fn+0x203/0x221 >> [ 63.980094] [<ffffffff81091f23>] ? smpboot_unpark_thread+0x33/0x33 >> [ 63.980094] [<ffffffff8108e44d>] kthread+0xc9/0xd1 >> [ 63.980094] [<ffffffff819e00ea>] ? do_wait_for_common+0xf8/0x125 >> [ 63.980094] [<ffffffff8108e384>] ? __kthread_parkme+0x61/0x61 >> [ 63.980094] [<ffffffff819e43ec>] ret_from_fork+0x7c/0xb0 >> [ 63.980094] [<ffffffff8108e384>] ? __kthread_parkme+0x61/0x61 >> >> tp could be freed in call_rcu callback too, the order is not guaranteed. >> >> Cc: John Fastabend <john.r.fastabend@...el.com> >> Signed-off-by: Cong Wang <xiyou.wangcong@...il.com> >> --- > > Thanks for catching this. What if we just drop tcf_exts_result > I can't see how its being used anymore. It appears to just be passed > around the ./net/sched files for some historic reason that is lost on > me. Would you mind testing a patch if I sent it out? > > Maybe Jamal can shed some light? > Sorry I should say its not needed to pass to the actions, tcf_exts_exec(). It _is_ needed here to get the class setup correct. And the tcf_exts_exec() stuff is a separate patch. Thanks again. Acked-by: John Fastabend <john.r.fastabend@...el.com> > >> include/net/pkt_cls.h | 6 +----- >> net/sched/cls_u32.c | 10 ++++++---- >> 2 files changed, 7 insertions(+), 9 deletions(-) >> >> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h >> index 73f9532..ef44ad9 100644 >> --- a/include/net/pkt_cls.h >> +++ b/include/net/pkt_cls.h >> @@ -20,11 +20,7 @@ int unregister_tcf_proto_ops(struct tcf_proto_ops >> *ops); >> static inline unsigned long >> __cls_set_class(unsigned long *clp, unsigned long cl) >> { >> - unsigned long old_cl; >> - >> - old_cl = *clp; >> - *clp = cl; >> - return old_cl; >> + return xchg(clp, cl); >> } >> >> static inline unsigned long >> diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c >> index 4be3ebf..0472909 100644 >> --- a/net/sched/cls_u32.c >> +++ b/net/sched/cls_u32.c >> @@ -358,7 +358,6 @@ static int u32_destroy_key(struct tcf_proto *tp, >> struct tc_u_knode *n, >> bool free_pf) >> { >> - tcf_unbind_filter(tp, &n->res); >> tcf_exts_destroy(&n->exts); >> if (n->ht_down) >> n->ht_down->refcnt--; >> @@ -416,6 +415,7 @@ static int u32_delete_key(struct tcf_proto *tp, >> struct tc_u_knode *key) >> if (pkp == key) { >> RCU_INIT_POINTER(*kp, key->next); >> >> + tcf_unbind_filter(tp, &key->res); >> call_rcu(&key->rcu, u32_delete_key_freepf_rcu); >> return 0; >> } >> @@ -425,7 +425,7 @@ static int u32_delete_key(struct tcf_proto *tp, >> struct tc_u_knode *key) >> return 0; >> } >> >> -static void u32_clear_hnode(struct tc_u_hnode *ht) >> +static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht) >> { >> struct tc_u_knode *n; >> unsigned int h; >> @@ -434,6 +434,7 @@ static void u32_clear_hnode(struct tc_u_hnode *ht) >> while ((n = rtnl_dereference(ht->ht[h])) != NULL) { >> RCU_INIT_POINTER(ht->ht[h], >> rtnl_dereference(n->next)); >> + tcf_unbind_filter(tp, &n->res); >> call_rcu(&n->rcu, u32_delete_key_freepf_rcu); >> } >> } >> @@ -447,7 +448,7 @@ static int u32_destroy_hnode(struct tcf_proto *tp, >> struct tc_u_hnode *ht) >> >> WARN_ON(ht->refcnt); >> >> - u32_clear_hnode(ht); >> + u32_clear_hnode(tp, ht); >> >> hn = &tp_c->hlist; >> for (phn = rtnl_dereference(*hn); >> @@ -482,7 +483,7 @@ static void u32_destroy(struct tcf_proto *tp) >> ht; >> ht = rtnl_dereference(ht->next)) { >> ht->refcnt--; >> - u32_clear_hnode(ht); >> + u32_clear_hnode(tp, ht); >> } >> >> while ((ht = rtnl_dereference(tp_c->hlist)) != NULL) { >> @@ -731,6 +732,7 @@ static int u32_change(struct net *net, struct >> sk_buff *in_skb, >> } >> >> u32_replace_knode(tp, tp_c, new); >> + tcf_unbind_filter(tp, &n->res); >> call_rcu(&n->rcu, u32_delete_key_rcu); >> return 0; >> } >> > > -- John Fastabend Intel Corporation -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists