From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jiri Pirko <jiri@resnulli.us>
Subject: Re: RCU callback crashes
Date: Wed, 20 Dec 2017 20:59:22 +0100
Message-ID: <20171220195922.GB1760@nanopsycho>
References: <20171219175921.7db9b0e1@cakuba.netronome.com>
 <20171220061118.GB1916@nanopsycho>
 <20171219222227.402e684a@cakuba.netronome.com>
 <20171219223404.03786d66@cakuba.netronome.com>
 <CAM_iQpWUjfv2-Sirmdb5WfV4pZ4uF0m7=HR5YGWaKxb4KHp8gQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Jakub Kicinski <kubakici@wp.pl>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
To: Cong Wang <xiyou.wangcong@gmail.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-wm0-f67.google.com ([74.125.82.67]:37322 "EHLO
        mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S932583AbdLTT7Y (ORCPT
        <rfc822;netdev@vger.kernel.org>); Wed, 20 Dec 2017 14:59:24 -0500
Received: by mail-wm0-f67.google.com with SMTP id f140so11911165wmd.2
        for <netdev@vger.kernel.org>; Wed, 20 Dec 2017 11:59:23 -0800 (PST)
Content-Disposition: inline
In-Reply-To: <CAM_iQpWUjfv2-Sirmdb5WfV4pZ4uF0m7=HR5YGWaKxb4KHp8gQ@mail.gmail.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Wed, Dec 20, 2017 at 07:17:50PM CET, xiyou.wangcong@gmail.com wrote:
>On Tue, Dec 19, 2017 at 10:34 PM, Jakub Kicinski <kubakici@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.

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.