netdev - Re: RCU callback crashes

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d82f1fdf-5d7b-db1b-6efb-85ff957f8a38@gmail.com>
Date:   Wed, 20 Dec 2017 12:14:49 -0800
From:   John Fastabend <john.fastabend@...il.com>
To:     Jiri Pirko <jiri@...nulli.us>, Cong Wang <xiyou.wangcong@...il.com>
Cc:     Jakub Kicinski <kubakici@...pl>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: RCU callback crashes

On 12/20/2017 11:59 AM, Jiri Pirko wrote:
> Wed, Dec 20, 2017 at 07:17:50PM CET, xiyou.wangcong@...il.com wrote:
>> On Tue, Dec 19, 2017 at 10:34 PM, Jakub Kicinski <kubakici@...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.
> 
> 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.
> 

Hi,

Just sent a patch to complete qdisc_destroy from rcu callback. This
is needed to resolve a race with the lockless qdisc patches.

But I guess it should fix the miniq issue as well?

Thanks,
John