From patchwork Wed Feb 17 17:01:11 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Rutland X-Patchwork-Id: 62123 Delivered-To: patch@linaro.org Received: by 10.112.43.199 with SMTP id y7csp113687lbl; Wed, 17 Feb 2016 09:03:03 -0800 (PST) X-Received: by 10.66.220.7 with SMTP id ps7mr3630255pac.58.1455728583757; Wed, 17 Feb 2016 09:03:03 -0800 (PST) Return-Path: Received: from bombadil.infradead.org (bombadil.infradead.org. [2001:1868:205::9]) by mx.google.com with ESMTPS id q8si2989675pfq.206.2016.02.17.09.03.03 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 17 Feb 2016 09:03:03 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) client-ip=2001:1868:205::9; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:1868:205::9 as permitted sender) smtp.mailfrom=linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aW5UJ-0003pf-6A; Wed, 17 Feb 2016 17:02:03 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aW5UE-0003Th-9f for linux-arm-kernel@lists.infradead.org; Wed, 17 Feb 2016 17:02:00 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 188813A8; Wed, 17 Feb 2016 09:00:53 -0800 (PST) Received: from leverpostej (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 91B7A3F21A; Wed, 17 Feb 2016 09:01:41 -0800 (PST) Date: Wed, 17 Feb 2016 17:01:11 +0000 From: Mark Rutland To: Andrey Ryabinin , Lorenzo Pieralisi , Catalin Marinas Subject: KASAN issues with idle / hotplug area (was: Re: [PATCH v5sub1 7/8] arm64: move kernel image to base of vmalloc area) Message-ID: <20160217170110.GE32647@leverpostej> References: <20160212151006.GJ31665@e104818-lin.cambridge.arm.com> <20160212152641.GK31665@e104818-lin.cambridge.arm.com> <56BDFC86.5010705@arm.com> <20160212160652.GL31665@e104818-lin.cambridge.arm.com> <56C1E072.2090909@virtuozzo.com> <20160215185957.GB19413@e104818-lin.cambridge.arm.com> <56C31D1D.50708@virtuozzo.com> <20160217143950.GC32647@leverpostej> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160217143950.GC32647@leverpostej> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160217_090158_513386_B226DB8B X-CRM114-Status: GOOD ( 25.71 ) X-Spam-Score: -6.9 (------) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (-6.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [217.140.101.70 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record -0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Ard Biesheuvel , Marc Zyngier , Will Deacon , Sudeep Holla , Jens Wiklander , "linux-arm-kernel@lists.infradead.org" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org On Wed, Feb 17, 2016 at 02:39:51PM +0000, Mark Rutland wrote: > When we go down for idle, in __cpu_suspend_enter we stash some context > to the stack (in assembly). The CPU may return from a cold state via > cpu_resume, where we restore context from the stack. > > However, after storing the context we call psci_suspend_finisher, which > calls psci_cpu_suspend, which calls invoke_psci_fn_*. As > psci_cpu_suspend and invoke_psci_fn_* are instrumented, they poison > memory on function entrance, but we never perform the unpoisoning. > > That was always the case for psci_suspend_finisher, so there was a > latent issue that we were somehow avoiding. Perhaps we got luck with > stack layout and never hit the poison. > > I'm not sure how we fix that, as invoke_psci_fn_* may or may not return > for arbitrary reasons (e.g. a CPU_SUSPEND_CALL may or may not return > depending on whether an interrupt comes in at the right time). > > Perhaps the simplest option is to not instrument invoke_psci_fn_* and > psci_suspend_finisher. Do we have a per-function annotation to avoid > KASAN instrumentation, like notrace? I need to investigate, but we may > also need notrace for similar reasons. I found __no_sanitize_address. As an aside, could we rename that to nokasan? That would match the style of notrace, is just as clear, and would make it far easier to write consistent legible function prototypes... Otherwise, I came up with the patch below, per the reasoning above. It _changes_ the KASAN splats (I see errors in tick_program_event rather than find_busiest_group), but doesn't seem to get rid of them. I'm not sure if I've missed something, or if we also have another latent issue. Ideas? Mark. ---->8---- >From 8f7ae44d8f8862f5300483d45617b5bd05fc652f Mon Sep 17 00:00:00 2001 From: Mark Rutland Date: Wed, 17 Feb 2016 15:38:22 +0000 Subject: [PATCH] arm64/psci: avoid KASAN splats with idle When a CPU goes into a deep idle state, we store CPU context in __cpu_suspend_enter, then call psci_suspend_finisher to invoke the firmware. If we entered a deep idle state, we do not return directly, and instead start cold, restoring state in cpu_resume. Thus we may execute the prologue and body of psci_suspend_finisher and the PSCI invocation function, but not their epilogue. When using KASAN this means that we poison a region of shadow memory, but never unpoison it. After we resume, subsequent stack accesses may hit the stale poison values, leading to false positives from KASAN. To avoid this, we must ensure that functions called after the context save are not instrumented, and do not posion the shadow region, by annotating them with __no_sanitize_address. As common inlines they may call are not similarly annotated, and the compiler refuses to allow function attribute mismatches, we must also avoid calls to such functions. ARM is not affected, as it does not support KASAN. When CONFIG_KASAN is not selected, __no_sanitize_address expands to nothing, so the annotation should not be harmful. Signed-off-by: Mark Rutland --- arch/arm64/kernel/psci.c | 14 ++++++++------ drivers/firmware/psci.c | 3 +++ 2 files changed, 11 insertions(+), 6 deletions(-) -- 1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c index f67f35b..8324ce8 100644 --- a/arch/arm64/kernel/psci.c +++ b/arch/arm64/kernel/psci.c @@ -32,12 +32,16 @@ static DEFINE_PER_CPU_READ_MOSTLY(u32 *, psci_power_state); +static phys_addr_t cpu_resume_phys; + static int __maybe_unused cpu_psci_cpu_init_idle(unsigned int cpu) { int i, ret, count = 0; u32 *psci_states; struct device_node *state_node, *cpu_node; + cpu_resume_phys = virt_to_phys(cpu_resume); + cpu_node = of_get_cpu_node(cpu, NULL); if (!cpu_node) return -ENODEV; @@ -178,12 +182,10 @@ static int cpu_psci_cpu_kill(unsigned int cpu) } #endif -static int psci_suspend_finisher(unsigned long index) +__no_sanitize_address +static int psci_suspend_finisher(unsigned long state) { - u32 *state = __this_cpu_read(psci_power_state); - - return psci_ops.cpu_suspend(state[index - 1], - virt_to_phys(cpu_resume)); + return psci_ops.cpu_suspend(state, cpu_resume_phys); } static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index) @@ -200,7 +202,7 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index) if (!psci_power_state_loses_context(state[index - 1])) ret = psci_ops.cpu_suspend(state[index - 1], 0); else - ret = cpu_suspend(index, psci_suspend_finisher); + ret = cpu_suspend(state[index - 1], psci_suspend_finisher); return ret; } diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c index f25cd79..e4e8dc1 100644 --- a/drivers/firmware/psci.c +++ b/drivers/firmware/psci.c @@ -106,6 +106,7 @@ bool psci_power_state_is_valid(u32 state) return !(state & ~valid_mask); } +__no_sanitize_address static unsigned long __invoke_psci_fn_hvc(unsigned long function_id, unsigned long arg0, unsigned long arg1, unsigned long arg2) @@ -116,6 +117,7 @@ static unsigned long __invoke_psci_fn_hvc(unsigned long function_id, return res.a0; } +__no_sanitize_address static unsigned long __invoke_psci_fn_smc(unsigned long function_id, unsigned long arg0, unsigned long arg1, unsigned long arg2) @@ -148,6 +150,7 @@ static u32 psci_get_version(void) return invoke_psci_fn(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0); } +__no_sanitize_address static int psci_cpu_suspend(u32 state, unsigned long entry_point) { int err;