On 12/20/2017 12:15 PM, Jiri Pirko wrote:
> Wed, Dec 20, 2017 at 08:59:22PM CET, j...@resnulli.us wrote:
>> Wed, Dec 20, 2017 at 07:17:50PM CET, xiyou.wangc...@gmail.com wrote:
>>> On Tue, Dec 19, 2017 at 10:34 PM, Jakub Kicinski <kubak...@wp.pl> wrote:
>>>> Ah, no object debug but KASAN on produces this:
>>>>
>>>
>>>
>>> I bet it is an ingress qdisc which is being freed?
>>>
>>>
>>>
>>>> [ 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 <john.fastab...@gmail.com>
> 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 <xiyou.wangc...@gmail.com>
Date: Tue Sep 19 13:15:42 2017 -0700
net_sched: no need to free qdisc in RCU callback
>
>>
>> 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.
