From patchwork Tue Mar 1 19:37:16 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Rutland X-Patchwork-Id: 63341 Delivered-To: patch@linaro.org Received: by 10.112.199.169 with SMTP id jl9csp2010493lbc; Tue, 1 Mar 2016 11:37:30 -0800 (PST) X-Received: by 10.66.222.199 with SMTP id qo7mr32798432pac.38.1456861050007; Tue, 01 Mar 2016 11:37:30 -0800 (PST) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id kr9si6706713pab.190.2016.03.01.11.37.29; Tue, 01 Mar 2016 11:37:29 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752446AbcCATh2 (ORCPT + 30 others); Tue, 1 Mar 2016 14:37:28 -0500 Received: from foss.arm.com ([217.140.101.70]:53865 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751417AbcCATh1 (ORCPT ); Tue, 1 Mar 2016 14:37:27 -0500 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 6F48049; Tue, 1 Mar 2016 11:36:31 -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 0821B3F25E; Tue, 1 Mar 2016 11:37:24 -0800 (PST) Date: Tue, 1 Mar 2016 19:37:16 +0000 From: Mark Rutland To: Alexander Potapenko Cc: Andrey Konovalov , Dmitriy Vyukov , Andrey Ryabinin , will.deacon@arm.com, catalin.marinas@arm.com, Andrew Morton , kasan-dev@googlegroups.com, LKML , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v1] kasan, arm64: Unpoison dirty stack frames when resuming from suspend. Message-ID: <20160301193529.GA335@leverpostej> References: <20160226135335.GC8728@leverpostej> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 26, 2016 at 06:28:27PM +0100, Alexander Potapenko wrote: > On Fri, Feb 26, 2016 at 2:53 PM, Mark Rutland wrote: > > Hi, > > > > On Fri, Feb 26, 2016 at 01:38:37PM +0100, Alexander Potapenko wrote: > >> Before an ARM64 CPU is suspended, the kernel saves the context which will > >> be used to initialize the register state upon resume. After that and > >> before the actual execution of the SMC instruction the kernel creates > >> several stack frames which are never unpoisoned because arm_smccc_smc() > >> does not return. This may cause false positive stack buffer overflow > >> reports from KASAN. > >> > >> The solution is to record the stack pointer value just before the CPU is > >> suspended, and unpoison the part of stack between the saved value and > >> the stack pointer upon resume. > > > > Thanks for looking into this! That's much appreciated. > > > > I think the general approach (unposioning the stack upon cold return to > > the kernel) is fine, but I have concerns with the implementation, which > > I've noted below. > > > > The problem also applies for hotplug, as leftover poison from the > > hot-unplug path isn't cleaned before a CPU is hotplugged back on. The > > first few functions are likely deterministic in their stack usage, so > > it's not seen with a defconfig, but I think it's possible to trigger, > > and it's also a cross-architecture problem shared with x86. > Agreed, but since I haven't yet seen problems with hotplug, it's hard > to test the fix for them. For testing, I used the below to deliberately hit stale poison after a hotplug. It deliberately creates large stack frames, accessing as much of the stack as possible to increase the chance of hitting any posion. Mark. ---->8---- diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c index 544a713..ef4693f 100644 --- a/kernel/sched/idle.c +++ b/kernel/sched/idle.c @@ -195,6 +195,21 @@ exit_idle: DEFINE_PER_CPU(bool, cpu_dead_idle); +#define NR_STACK_ELEMS 128 +static noinline void hit_stale_poison(unsigned int frames) +{ + volatile unsigned long magic[NR_STACK_ELEMS]; + int i; + + for (i = 0; i < NR_STACK_ELEMS; i++) + magic[i] = 0; + + if (frames) + hit_stale_poison(frames - 1); + + return; +} + /* * Generic idle loop implementation * @@ -202,6 +217,8 @@ DEFINE_PER_CPU(bool, cpu_dead_idle); */ static void cpu_idle_loop(void) { + hit_stale_poison(4); + while (1) { /* * If the arch has a polling bit, we maintain an invariant: