From patchwork Tue Dec 5 14:56:42 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Martin X-Patchwork-Id: 10093095 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 DE0FC6035E for ; Tue, 5 Dec 2017 14:57:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C8F9C296D8 for ; Tue, 5 Dec 2017 14:57:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BD3BB296F4; Tue, 5 Dec 2017 14:57:21 +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=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.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 C94AF296D8 for ; Tue, 5 Dec 2017 14:57:20 +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:MIME-Version:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:Message-Id:Date: Subject:To:From:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To: References:List-Owner; bh=Vdj4eVdkCKTbW6ZhKSmxv001v2hTDMZnuiYvqWvv4Yo=; b=CzP 6ocRz6h+9gccKIezam+rSvAmLgY0mS0WTtX98QV7+phUgoQ3ROLVomRz6R4T56ypvzkPxFgZiw2eu vfHAbUiRYQWQePpcqSQ5rq0G6OHsENB21VWoIMQq+/kwvdg/CslWxrXs2VNceqo5s03Mg9py5GXdk czQYnOapMc6nTVqa0sSNUdIwycX0cia6q3Ru1Cw7a0dBVyj/WPVX82ynSHFeydQW0edlZIvw/cc8Y PS10iUah6pOMjt//xDHzyd+cG7SH3TYtOaRYoukgYTkEixDscKKQJP5Jo+9hjRL5c+08VgDh9+Y63 HXpP38AYuzwXyis2c+z50OKG4Kp40Qw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1eMEeu-0001ac-BF; Tue, 05 Dec 2017 14:57:20 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1eMEel-0001QN-AL for linux-arm-kernel@lists.infradead.org; Tue, 05 Dec 2017 14:57:17 +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 E9C7C1529; Tue, 5 Dec 2017 06:56:50 -0800 (PST) Received: from e103592.cambridge.arm.com (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 749193F236; Tue, 5 Dec 2017 06:56:50 -0800 (PST) From: Dave Martin To: linux-arm-kernel@lists.infradead.org Subject: [PATCH] arm64: fpsimd: Prevent registers leaking from dead tasks Date: Tue, 5 Dec 2017 14:56:42 +0000 Message-Id: <1512485802-29489-1-git-send-email-Dave.Martin@arm.com> X-Mailer: git-send-email 2.1.4 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20171205_065711_552397_DD40383B X-CRM114-Status: GOOD ( 11.83 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Ard Biesheuvel MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Currently, loading of a task's fpsimd state into the CPU registers is skipped if that task's state is already present in the registers of that CPU. However, the code relies on the struct fpsimd_state * (and by extension struct task_struct *) to unambiguously identify a task. There is a particular case in which this doesn't work reliably: when a task exits, its task_struct may be recycled to describe a new task. Consider the following scenario: 1) Task P loads its fpsimd state onto cpu C. per_cpu(fpsimd_last_state, C) := P; P->thread.fpsimd_state.cpu := C; 2) Task X is scheduled onto C and loads its fpsimd state on C. per_cpu(fpsimd_last_state, C) := X; X->thread.fpsimd_state.cpu := C; 3) X exits, causing X's task_struct to be freed. 4) P forks a new child T, which obtains X's recycled task_struct. T == X. T->thread.fpsimd_state.cpu == C (inherited from P). 5) T is scheduled on C. T's fpsimd state is not loaded, because per_cpu(fpsimd_last_state, C) == T (== X) && T->thread.fpsimd_state.cpu == C. (This is the check performed by fpsimd_thread_switch().) So, T gets X's registers because the last registers loaded onto C were those of X, in (2). This patch fixes the problem by ensuring that the sched-in check fails in (5): fpsimd_flush_task_state(T) is called when T is forked, so that T->thread.fpsimd_state.cpu == C cannot be true. This relies on the fact that T is not schedulable until after copy_thread() completes. Once T's fpsimd state has been loaded on some CPU C there may still be other cpus D for which per_cpu(fpsimd_last_state, D) == &X->thread.fpsimd_state. But D is necessarily != C in this case, and the check in (5) must fail. An alternative fix would be to do refcounting on task_struct. This would result in each CPU holding a reference to the last task whose fpsimd state was loaded there. It's not clear whether this is preferable, and it involves higher overhead than the fix proposed in this patch. It would also moves all the task_struct freeing work into the context switch critical section, or otherwise some deferred cleanup mechanism would need to be introduced, neither of which seems obviously justified. Fixes: 005f78cd8849 ("arm64: defer reloading a task's FPSIMD state to userland resume") Signed-off-by: Dave Martin Cc: Ard Biesheuvel Reviewed-by: Ard Biesheuvel --- arch/arm64/kernel/process.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index b2adcce..5387d15 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -313,6 +313,7 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start, */ clear_tsk_thread_flag(p, TIF_SVE); p->thread.sve_state = NULL; + fpsimd_flush_task_state(p); if (likely(!(p->flags & PF_KTHREAD))) { *childregs = *current_pt_regs();