From patchwork Fri Mar 30 08:08:12 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ji Zhang X-Patchwork-Id: 10317357 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0E2C46063D for ; Fri, 30 Mar 2018 08:08:39 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 00AA22A528 for ; Fri, 30 Mar 2018 08:08:39 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E91812A532; Fri, 30 Mar 2018 08:08:38 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 9B59B2A528 for ; Fri, 30 Mar 2018 08:08:37 +0000 (UTC) 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:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=6HDjf7EI/PkPBUfV4GO4fMvCMhoWszVhe0Jwb29dpOk=; b=Ae9+w+Y6gQ+DyO 92X+qW55wnDV0DRyojv//c0n9nHUCbz7U6b++6A0f3yWnf+gbA2ASQ6Uf7NGqh7V5wK+x4ck9qtmm 9O5b401lDNE5op8vBfrLcWJjE55lFaIovwALfJQ7caU+bIEhg9+CLOnamtupFAHF5cV8NkBzSXtXn izpU2jULbJgISynLqvZuHbBVEi3ge1rAkLuSb3qdGadyIM8CPImx77ZFEa5E+3XPYFaV9EgtwDXvy 1+cf1UeLomF55wDYEDjL8esqWEMYQp8Q64QJmtxxO77E5tTU+iiN3zJ9Gz8tsID1VDC4z5N8si5BH V9A8e4GkoTFDNJdae6Nw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1f1p5Q-0005Nn-KA; Fri, 30 Mar 2018 08:08:36 +0000 Received: from [210.61.82.184] (helo=mailgw02.mediatek.com) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1f1p5N-0005LY-Gf; Fri, 30 Mar 2018 08:08:35 +0000 X-UUID: 15b425541070494fa1c180d101f91e59-20180330 Received: from mtkcas07.mediatek.inc [(172.21.101.84)] by mailgw02.mediatek.com (envelope-from ) (mhqrelay.mediatek.com ESMTP with TLS) with ESMTP id 1317607845; Fri, 30 Mar 2018 16:08:14 +0800 Received: from mtkcas07.mediatek.inc (172.21.101.84) by mtkmbs03n1.mediatek.inc (172.21.101.181) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Fri, 30 Mar 2018 16:08:12 +0800 Received: from [172.21.84.99] (172.21.84.99) by mtkcas07.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1210.3 via Frontend Transport; Fri, 30 Mar 2018 16:08:12 +0800 Message-ID: <1522397292.26617.63.camel@mtksdccf07> Subject: Re: [PATCH] arm64: avoid race condition issue in dump_backtrace From: Ji.Zhang To: Mark Rutland Date: Fri, 30 Mar 2018 16:08:12 +0800 In-Reply-To: <20180328101240.moo44g5qd3qjuxgn@lakrids.cambridge.arm.com> References: <1521687960-3744-1-git-send-email-ji.zhang@mediatek.com> <20180322055929.z25brvwlmdighz66@salmiak> <1521711329.26617.31.camel@mtksdccf07> <20180326113932.2i6qp3776jtmcqk4@lakrids.cambridge.arm.com> <1522229612.26617.47.camel@mtksdccf07> <20180328101240.moo44g5qd3qjuxgn@lakrids.cambridge.arm.com> X-Mailer: Evolution 3.2.3-0ubuntu6 MIME-Version: 1.0 X-TM-AS-Product-Ver: SMEX-12.5.0.5042-8.2.9001-23750.005 X-TM-AS-Result: No-7.534000-8.000000-10 X-TMASE-MatchedRID: QW5G6BKkLToOwH4pD14DsPHkpkyUphL9QrO4XR6BRQO2F4a+vI22PgTm MKJk1G07/lTC/csg67JhrA+CokynbwjdAxZ9icLjA9lly13c/gGVq+okl1rYDzImRMXd9uwUB7X WBXOexYVanfi7p7YrsdrVs7ejcEropJu8vlDMnojcWo5Vvs8MQkCrr/LkAQ46zQ05wK4VbrXSVI EgNsIq53nFQv7ffIYFvXDnY3P+N5U3+hnFgqdP4hlckvO1m+JcVG1q2Q/KGL6en0qBdy7fjPJIH smNW7izRNg+Ky3/qcWRk6XtYogiau9c69BWUTGwC24oEZ6SpSkj80Za3RRg8M1X87JdYOdgzoXH Nvxv/CE4O3wVb6RoTOEb32VtNbcyWtV+WcZO8f0= X-TM-AS-User-Approved-Sender: No X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--7.534000-8.000000 X-TMASE-Version: SMEX-12.5.0.5042-8.2.9001-23750.005 X-TMASE-POSTMAN: 2-d; X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20180330_010833_814967_BE429DAA X-CRM114-Status: GOOD ( 18.54 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: wsd_upstream@mediatek.com, Xie XiuQi , Ard Biesheuvel , Marc Zyngier , Catalin Marinas , Julien Thierry , Will Deacon , linux-kernel@vger.kernel.org, shadanji@163.com, James Morse , Matthias Brugger , linux-mediatek@lists.infradead.org, Michael Weiser , Dave Martin , linux-arm-kernel@lists.infradead.org Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+patchwork-linux-mediatek=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, 2018-03-28 at 11:12 +0100, Mark Rutland wrote: > On Wed, Mar 28, 2018 at 05:33:32PM +0800, Ji.Zhang wrote: > > I'm very much not keen on this. > > I think that if we're going to do this, the only sane way to do it is to > have unwind_frame() verify the current fp against the previous one, and > verify that we have some strict nesting of stacks. Generally, that means > we can go: > > overflow -> irq -> task > > ... though I'm not sure what to do about the SDEI stack vs the overflow > stack. Actually I have had the fp check in unwind_frame(), but since I use the in_entry_text() to determine if stack spans, and I did not want to include traps.h in stacktrace.c, so I move the check out to dump_backtrace. Anyway, It seems that the key point is how should we verify that there are some nesting of stacks. Since in unwind_frame() we already have the previous fp and current fp, could we assume that if these two fps are NOT belong to the same stack, there should be stack spans (no matter task->irq, or irq->overflow, etc), and we can do the tricky to bypass the fp check.The sample of the prosal just like: (frame->pc == (unsigned long)return_to_handler)) { Could this work? Best Regards, Ji diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h index 902f9ed..fc2bf4d 100644 --- a/arch/arm64/include/asm/stacktrace.h +++ b/arch/arm64/include/asm/stacktrace.h @@ -92,4 +92,18 @@ static inline bool on_accessible_stack(struct task_struct *tsk, unsigned long sp return false; } +static inline bool on_same_stack(struct task_struct *tsk, unsigned long sp1, unsigned sp2) +{ + if (on_task_stack(tsk, sp1) && on_task_stack(tsk, sp2)) + return true; + if (on_irq_stack(sp1) && on_irq_stack(sp2)) + return true; + if (on_overflow_stack(sp1) && on_overflow_stack(sp2)) + return true; + if (on_sdei_stack(sp1) && on_sdei_stack(sp2)) + return true; + + return false; +} + #endif /* __ASM_STACKTRACE_H */ diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c index d5718a0..4907a67 100644 --- a/arch/arm64/kernel/stacktrace.c +++ b/arch/arm64/kernel/stacktrace.c @@ -56,6 +56,13 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame) frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp)); frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8)); + if (!on_same_stack(fp, frame->fp)) + fp = frame->fp + 0x8; + if (fp <= frame->fp) { + pr_notice("fp invalid, stop unwind\n"); + return -EINVAL; + } + #ifdef CONFIG_FUNCTION_GRAPH_TRACER if (tsk->ret_stack &&