From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.7 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 019E0C2D0F8 for ; Wed, 13 May 2020 03:19:56 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B50FF20714 for ; Wed, 13 May 2020 03:19:55 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="of8CbmHX"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="kdXcrlFV" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B50FF20714 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:Reply-To:List-Subscribe:List-Help: List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version: References:Message-ID:Subject:To:From:Date:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=sn14Wkne2pgpvYxV7+omylPM/Njn9vYj6pey5N9o7e4=; b=of8CbmHXKF/mt9 vpIIUcdG1cGc95qjNZbAuDB83aLduJfdg3hOdzcfTFCCJOxDg3h1LzUhUkwNH2AMw7m4phsB7E+U1 N6JnEwMpcidkZaRqazSKwUFU8tRbYpFgB69bbLhwmJr+Tro9pbPdcrgY9r3RigC1xS1TYbYhVxpjS ea/ExWbw4TrQ6meQr9BXbPrrk1gBvJZWDayABRXlJbwMGznJLaiRu5iGwVzMXXJ7xfL32vbWO8rZW L5RWvdK2xkMA2j/3PMqU3hx7hNWihLqrd5LRLeCxHesyMHqz3h9w/T+ND41nk+j8GVMJvqiq7rKFD HKNZ2+BimMZPinUiMIrw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jYhw3-0001gT-9z; Wed, 13 May 2020 03:19:55 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jYhw0-0001dT-DQ; Wed, 13 May 2020 03:19:53 +0000 Received: from paulmck-ThinkPad-P72.home (unknown [50.39.105.78]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 4CA0420714; Wed, 13 May 2020 03:19:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1589339991; bh=3evM/3WoUY87UjYJg6kocy7JRyvcnZJAhsOd5UgvkMQ=; h=Date:From:To:Cc:Subject:Reply-To:References:In-Reply-To:From; b=kdXcrlFVRId5W4tSKz2O+7bJSB24kuk9EK2RoEBms3JY4jhtAH4wBk2NsC/fRBGq+ Y4Vkyn2Yu/0NnQy3N3V0LHNZIbyxfMJdFmAcjqNo9jQqJYpn+TCbdNVRvzlSi43+Uo zbhmsSWI4UQWFdk6BmTRCRPg7o/LaluyBQfj/GVE= Received: by paulmck-ThinkPad-P72.home (Postfix, from userid 1000) id 349373523471; Tue, 12 May 2020 20:19:51 -0700 (PDT) Date: Tue, 12 May 2020 20:19:51 -0700 From: "Paul E. McKenney" To: Walter Wu Subject: Re: [PATCH v2 1/3] rcu/kasan: record and print call_rcu() call stack Message-ID: <20200513031951.GO2869@paulmck-ThinkPad-P72> References: <20200511023111.15310-1-walter-zh.wu@mediatek.com> <20200511180527.GZ2869@paulmck-ThinkPad-P72> <1589250993.19238.22.camel@mtksdccf07> <20200512142541.GD2869@paulmck-ThinkPad-P72> <20200512161422.GG2869@paulmck-ThinkPad-P72> <1589335531.19238.52.camel@mtksdccf07> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1589335531.19238.52.camel@mtksdccf07> User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200512_201952_495389_FED630F7 X-CRM114-Status: GOOD ( 31.46 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: paulmck@kernel.org Cc: wsd_upstream , Linux-MM , Lai Jiangshan , Josh Triplett , kasan-dev , LKML , Joel Fernandes , linux-mediatek@lists.infradead.org, Alexander Potapenko , Linux ARM , Matthias Brugger , Andrey Ryabinin , Andrew Morton , Dmitry Vyukov , Mathieu Desnoyers Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, May 13, 2020 at 10:05:31AM +0800, Walter Wu wrote: > On Tue, 2020-05-12 at 18:22 +0200, Dmitry Vyukov wrote: > > On Tue, May 12, 2020 at 6:14 PM Paul E. McKenney wrote: > > > > > > > > > This feature will record first and last call_rcu() call stack and > > > > > > > > > print two call_rcu() call stack in KASAN report. > > > > > > > > > > > > > > > > Suppose that a given rcu_head structure is passed to call_rcu(), then > > > > > > > > the grace period elapses, the callback is invoked, and the enclosing > > > > > > > > data structure is freed. But then that same region of memory is > > > > > > > > immediately reallocated as the same type of structure and again > > > > > > > > passed to call_rcu(), and that this cycle repeats several times. > > > > > > > > > > > > > > > > Would the first call stack forever be associated with the first > > > > > > > > call_rcu() in this series? If so, wouldn't the last two usually > > > > > > > > be the most useful? Or am I unclear on the use case? > > > > > > > > > > > > 2 points here: > > > > > > > > > > > > 1. With KASAN the object won't be immediately reallocated. KASAN has > > > > > > 'quarantine' to delay reuse of heap objects. It is assumed that the > > > > > > object is still in quarantine when we detect a use-after-free. In such > > > > > > a case we will have proper call_rcu stacks as well. > > > > > > It is possible that the object is not in quarantine already and was > > > > > > reused several times (quarantine is not infinite), but then KASAN will > > > > > > report non-sense stacks for allocation/free as well. So wrong call_rcu > > > > > > stacks are less of a problem in such cases. > > > > > > > > > > > > 2. We would like to memorize 2 last call_rcu stacks regardless, but we > > > > > > just don't have a good place for the index (bit which of the 2 is the > > > > > > one to overwrite). Probably could shove it into some existing field, > > > > > > but then will require atomic operations, etc. > > > > > > > > > > > > Nobody knows how well/bad it will work. I think we need to get the > > > > > > first version in, deploy on syzbot, accumulate some base of example > > > > > > reports and iterate from there. > > > > > > > > > > If I understood the stack-index point below, why not just move the > > > > > previous stackm index to clobber the previous-to-previous stack index, > > > > > then put the current stack index into the spot thus opened up? > > > > > > > > We don't have any index in this change (don't have memory for such index). > > > > The pseudo code is" > > > > > > > > u32 aux_stacks[2]; // = {0,0} > > > > > > > > if (aux_stacks[0] != 0) > > > > aux_stacks[0] = stack; > > > > else > > > > aux_stacks[1] = stack; > > > > > > I was thinking in terms of something like this: > > > > > > u32 aux_stacks[2]; // = {0,0} > > > > > > if (aux_stacks[0] != 0) { > > > aux_stacks[0] = stack; > > > } else { > > > if (aux_stacks[1]) > > > aux_stacks[0] = aux_stacks[1]; > > > aux_stacks[1] = stack; > > > } > > > > > > Whether this actually makes sense in real life, I have no idea. > > > The theory is that you want the last two stacks. However, if these > > > elements get cleared at kfree() time, then I could easily believe that > > > the approach you already have (first and last) is the way to go. > > > > > > Just asking the question, not arguing for a change! > > > > Oh, this is so obvious... in hindsight! :) > > > > Walter, what do you think? > > > > u32 aux_stacks[2]; // = {0,0} > > if (aux_stacks[0] != 0) { > aux_stacks[0] = stack; > } else { > if (aux_stacks[1]) > aux_stacks[0] = aux_stacks[1]; > aux_stacks[1] = stack; > } > > Hmm...why I think it will always cover aux_stacks[0] after aux_stacks[0] > has stack, it should not record last two stacks? > > How about this: > > u32 aux_stacks[2]; // = {0,0} > > if (aux_stacks[1]) > aux_stacks[0] = aux_stacks[1]; > aux_stacks[1] = stack; Even better! ;-) Thanx, Paul > > I would do this. I think latter stacks are generally more interesting > > wrt shedding light on a bug. The first stack may even be "statically > > known" (e.g. if object is always queued into a workqueue for some lazy > > initialization during construction). > > I think it make more sense to record latter stack, too. > > Thanks for your and Paul's suggestion. > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel