From patchwork Thu Dec 3 17:27:08 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 7762051 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 94F42BEEE1 for ; Thu, 3 Dec 2015 17:31:38 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5FBC5204EC for ; Thu, 3 Dec 2015 17:31:37 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 41E75204E7 for ; Thu, 3 Dec 2015 17:31:36 +0000 (UTC) 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 1a4XhT-0004p7-Tc; Thu, 03 Dec 2015 17:29:47 +0000 Received: from pandora.arm.linux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1a4XhP-0004P8-3K for linux-arm-kernel@lists.infradead.org; Thu, 03 Dec 2015 17:29:45 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=BhAjAFAw+DlCIQF1lSk2ohM8WGoH9goXKzebgLqY3Po=; b=TGqGGe+x/tSZMRoT+cFk1C6nrNzW3CxSugbeIife0Yfw5OjMHkNtLSl2ZbKBcCGxdPWxSnOtd5atszlByvVvKNR828P8cBCIdtRA1VWlzLVIB30IYd76ZWw8axTH6+VQST3aGHlBFIFyXZl4dcrhvfMvgH8qCjHS+qWS+1KkWM0=; Received: from n2100.arm.linux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:4f86]:42974) by pandora.arm.linux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1a4Xey-0008Jl-Tt; Thu, 03 Dec 2015 17:27:13 +0000 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1a4Xev-0007BB-96; Thu, 03 Dec 2015 17:27:09 +0000 Date: Thu, 3 Dec 2015 17:27:08 +0000 From: Russell King - ARM Linux To: Peter Rosin Subject: Re: Domain faults when CONFIG_CPU_SW_DOMAIN_PAN is enabled Message-ID: <20151203172708.GT8644@n2100.arm.linux.org.uk> References: <1267b061b66c4d1fa8af1955825bc97a@EMAIL.axentia.se> <20151203110041.GK8644@n2100.arm.linux.org.uk> <20151203115143.GM8644@n2100.arm.linux.org.uk> <533b0f1f264e42e78b94bbff9570fb50@EMAIL.axentia.se> <20151203133727.GN8644@n2100.arm.linux.org.uk> <94580382ca344ae2b64f66fb778c6ff7@EMAIL.axentia.se> <20151203164118.GR8644@n2100.arm.linux.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20151203164118.GR8644@n2100.arm.linux.org.uk> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20151203_092943_587926_7BBD62A9 X-CRM114-Status: GOOD ( 23.26 ) X-Spam-Score: -4.3 (----) 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: Will Deacon , "'linux-kernel@vger.kernel.org'" , "'linux-arm-kernel@lists.infradead.org'" , "nico@fluxnic.net" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Dec 03, 2015 at 04:41:18PM +0000, Russell King - ARM Linux wrote: > On Thu, Dec 03, 2015 at 04:12:06PM +0000, Peter Rosin wrote: > > * uaccess_with_memcpy.c:__copy_to_user() has a mode in which it copies > > "non-atomically" (if faulthandler_disabled() returns 0). If a fault > > happens during __copy_to_user, what prevents some other thread from > > clobbering DACR? > > See the second point above. Moreover, if we sleep in down_read(), > then __switch_to() reads the current DACR value and saves it in the > thread information, and will restore that value when resuming the > thread - even if the thread has been migrated to a different CPU. I thought this was correct, but it isn't - that's what my original solution did, but I think when Will reviewed it, we decided it wasn't necessary - and it isn't necessary for every single case with the exception of this one. This is exactly what's going wrong: the down_read() in these paths calls into the scheduler, which switches away. When we come back, the DACR value is reset by the other thread to 0x51. There's a few ways to solve this: 1. Make the thread switching code save and restore the DACR register as it would do for domains. This imposes an overhead on every single context switch whether or not we happen to be in this _single_ troublesome code. (Patch attached - as there's several, I'm attaching them.) 2. Add additional code to the uaccess-with-memcpy stuff to reset the DACR value prior to using memcpy() or memset(). (Patch attached.) 3. Make uaccess-with-memcpy depend on !CPU_SW_DOMAINS_PAN (suggested by Will) 4. Delete the uaccess-with-memcpy code (also suggested by Will.) I think the best thing I can do is say... "Discuss amongst yourselves" :) Reported-by: Peter Rosin Tested-by: Peter Rosin diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S index 3ce377f7251f..ae8a3ad763d9 100644 --- a/arch/arm/kernel/entry-armv.S +++ b/arch/arm/kernel/entry-armv.S @@ -782,7 +782,7 @@ ENTRY(__switch_to) THUMB( str lr, [ip], #4 ) ldr r4, [r2, #TI_TP_VALUE] ldr r5, [r2, #TI_TP_VALUE + 4] -#ifdef CONFIG_CPU_USE_DOMAINS +#if defined(CONFIG_CPU_USE_DOMAINS) || defined(CONFIG_CPU_SW_DOMAIN_PAN) mrc p15, 0, r6, c3, c0, 0 @ Get domain register str r6, [r1, #TI_CPU_DOMAIN] @ Save old domain register ldr r6, [r2, #TI_CPU_DOMAIN] @@ -793,7 +793,7 @@ ENTRY(__switch_to) ldr r8, =__stack_chk_guard ldr r7, [r7, #TSK_STACK_CANARY] #endif -#ifdef CONFIG_CPU_USE_DOMAINS +#if defined(CONFIG_CPU_USE_DOMAINS) || defined(CONFIG_CPU_SW_DOMAIN_PAN) mcr p15, 0, r6, c3, c0, 0 @ Set domain register #endif mov r5, r0 diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c index 4adfb46e3ee9..9d80eb20488f 100644 --- a/arch/arm/kernel/process.c +++ b/arch/arm/kernel/process.c @@ -229,7 +229,7 @@ copy_thread(unsigned long clone_flags, unsigned long stack_start, memset(&thread->cpu_context, 0, sizeof(struct cpu_context_save)); -#ifdef CONFIG_CPU_USE_DOMAINS +#if defined(CONFIG_CPU_USE_DOMAINS) || defined(CONFIG_CPU_SW_DOMAIN_PAN) /* * Copy the initial value of the domain access control register * from the current thread: thread->addr_limit will have been