Re: RCU callback crashes

On 12/20/2017 12:15 PM, Jiri Pirko wrote:

Wed, Dec 20, 2017 at 08:59:22PM CET, jiri@resnulli.us wrote:

quoted

Wed, Dec 20, 2017 at 07:17:50PM CET, xiyou.wangcong@gmail.com wrote:

quoted

On Tue, Dec 19, 2017 at 10:34 PM, Jakub Kicinski [off-list ref] wrote:

quoted

Ah, no object debug but KASAN on produces this:

I bet it is an ingress qdisc which is being freed?

quoted

[   39.268209] BUG: KASAN: use-after-free in cpu_needs_another_gp+0x246/0x2b0
[   39.275965] Read of size 8 at addr ffff8803aa64f138 by task swapper/13/0
[   39.283524]
[   39.285256] CPU: 13 PID: 0 Comm: swapper/13 Not tainted 4.15.0-rc3-perf-00955-g1d0b01347dd5-dirty #8
[   39.295535] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.3.4 11/08/2016
[   39.303969] Call Trace:
[   39.306769]  <IRQ>
[   39.309088]  dump_stack+0xa6/0x118
[   39.312957]  ? _atomic_dec_and_lock+0xe8/0xe8
[   39.317895]  ? cpu_needs_another_gp+0x246/0x2b0
[   39.323030]  print_address_description+0x6a/0x270
[   39.328380]  ? cpu_needs_another_gp+0x246/0x2b0
[   39.333510]  kasan_report+0x23f/0x350
[   39.337672]  cpu_needs_another_gp+0x246/0x2b0
...
[   39.383026]  rcu_process_callbacks+0x1a0/0x620
...

This is confusing.

I guess it is q->miniqp which is freed in qdisc_graft() without properly
waiting for rcu readers?

miniqp is inside qdisc private data:
struct ingress_sched_data {
       struct tcf_block *block;
       struct tcf_block_ext_info block_info;
       struct mini_Qdisc_pair miniqp;
};

That is freed along with the qdisc itself in:
qdisc_destroy->qdisc_free

Before miniq, tp was checked in the rcu reader path. In case it was not
null, q was processed. In slow patch, tp is freed after rcu grace period:
tcf_proto_destroy->kfree_rcu

I assumed that since q is processed in rcu reader, it is also freed after
a grace period, but now looking at the code I don't see it happening
like that.

Aha! It was removed by:
commit c5ad119fb6c09b0297446be05bd66602fa564758
Author: John Fastabend [off-list ref]
Date:   Thu Dec 7 09:58:19 2017 -0800

    net: sched: pfifo_fast use skb_array

Even farther back right,

commit 752fbcc33405d6f8249465e4b2c4e420091bb825
Author: Cong Wang [off-list ref]
Date:   Tue Sep 19 13:15:42 2017 -0700

    net_sched: no need to free qdisc in RCU callback

quoted

So I think that change to miniq made the existing race window
a bit wider and easier to hit.

I believe that calling kfree_rcu by call_rcu should resolve this.