From patchwork Tue Jun 19 14:22:05 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexey Brodkin X-Patchwork-Id: 931636 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.infradead.org (client-ip=2607:7c80:54:e::133; helo=bombadil.infradead.org; envelope-from=linux-snps-arc-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=synopsys.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="e9IhibU0"; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:e::133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4199CV4VZTz9ryk for ; Wed, 20 Jun 2018 00:22:30 +1000 (AEST) 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=FwJGcf6opRJb3zmlgm0xfsR09UI9zPZLxa78BXvtfi8=; b=e9I hibU0bLCjqlRJqwlhTMqK9L9ew7KYAkcG80OFss997Hc9fL3QJTXjVRnb0lL2GM4hbmTKDdwSjJWS 5NZUZKP6gYD9NfvwRmxb/JgDD8szekK0t/FdjyjWezr7p4qLg9Sjgk7KeruFJkHrCo5ctoRbda7ne rfyLUWKjoP5JyMFELMNOOLhd71HmoF3R8DmlHYtygL5O1OzNfA59cCYaSPUDtdzvNRKRYrUXYCDfv 58y7lsPRNGcO7Wy1RRMbt5n9SNJeFqVT0+phiWljqs8IPdcN0Dljp9G5RYMD3tIcfwxUfwHgPZVkw 9A67Q9EQdmaE0CpyJsuf6GkOhl41uJA==; 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 1fVHWd-0003kq-QP; Tue, 19 Jun 2018 14:22:27 +0000 Received: from smtprelay.synopsys.com ([198.182.47.9]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1fVHWY-0003gS-0w for linux-snps-arc@lists.infradead.org; Tue, 19 Jun 2018 14:22:25 +0000 Received: from mailhost.synopsys.com (mailhost1.synopsys.com [10.12.238.239]) by smtprelay.synopsys.com (Postfix) with ESMTP id B68D224E22D1; Tue, 19 Jun 2018 07:22:10 -0700 (PDT) Received: from abrodkin-7480l.internal.synopsys.com (unknown [10.121.8.87]) by mailhost.synopsys.com (Postfix) with ESMTP id 98A885E09; Tue, 19 Jun 2018 07:22:08 -0700 (PDT) From: Alexey Brodkin To: linux-snps-arc@lists.infradead.org Subject: [PATCH v2] ARC: Improve cmpxchg syscall implementation Date: Tue, 19 Jun 2018 17:22:05 +0300 Message-Id: <20180619142205.20493-1-abrodkin@synopsys.com> X-Mailer: git-send-email 2.17.1 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20180619_072222_084949_782104BB X-CRM114-Status: GOOD ( 16.52 ) X-Spam-Score: -0.0 (/) X-Spam-Report: SpamAssassin version 3.4.1 on bombadil.infradead.org summary: Content analysis details: (-0.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [198.182.47.9 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record X-BeenThere: linux-snps-arc@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: Linux on Synopsys ARC Processors List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-arch@vger.kernel.org, Peter Zijlstra , Vineet Gupta , Alexey Brodkin , linux-kernel@vger.kernel.org, Max Filippov MIME-Version: 1.0 Sender: "linux-snps-arc" Errors-To: linux-snps-arc-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org From: Peter Zijlstra arc_usr_cmpxchg syscall is supposed to be used on platforms that lack support of Load-Locked/Store-Conditional instructions in hardware. And in that case we mimic missing hardware features with help of kernel's sycall that "atomically" checks current value in memory and then if it matches caller expectation new value is written to that same location. What's important in the description above: - Check-and-exchange must be "atomical" which means preemption must be disabled during entire "transaction" - Data accessed is from user-space, i.e. we're dealing with virtual addresses And in current implementation we have a couple of problems: 1. We do disable preemprion around __get_user() & __put_user() but that in its turn disables page fault handler. That means if a pointer to user's data has no mapping in the TLB we won't be able to access required data. Instead software "exception handling" code from __get_user_fn() will return -EFAULT. 2. What's worse if we're dealing with data from not yet allocated page (think of pre-copy-on-write state) we'll successfully read data but on write we'll silently return to user-space with correct result (which we really read just before). That leads to very strange problems in user-space app further down the line because new value was never written to the destination. 3. Regardless of what went wrong we'll return from syscall and user-space application will continue to execute. Even if user's pointer was completely bogus. In case of hardware LL/SC that app would have been killed by the kernel. With that change we attempt to imrove on all 3 items above: 1. We still disable preemption around write of user's data but if we happen to fail with write we're enabling preemption and try to fix-up page fault so that we have a correct permission for writing user's data. Then re-try again in "atomic" context. 2. If real page fault fails or even access_ok() returns false we send SIGSEGV to the user-space process so if something goes seriously wrong we'll know about it much earlier. Signed-off-by: Peter Zijlstra Signed-off-by: Alexey Brodkin Cc: Vineet Gupta Cc: Max Filippov Cc: linux-arch@vger.kernel.org --- Changes v1 -> v2: * Peter's almost clean-room reimplmentation with less paranoid checks and direct invocation of fixup_user_fault() for in-place update of write permissions. arch/arc/kernel/process.c | 48 ++++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/arch/arc/kernel/process.c b/arch/arc/kernel/process.c index 5ac3b547453f..7a7742fba77a 100644 --- a/arch/arc/kernel/process.c +++ b/arch/arc/kernel/process.c @@ -47,7 +47,9 @@ SYSCALL_DEFINE0(arc_gettls) SYSCALL_DEFINE3(arc_usr_cmpxchg, int *, uaddr, int, expected, int, new) { struct pt_regs *regs = current_pt_regs(); - int uval = -EFAULT; + struct page *page; + u32 val; + int ret; /* * This is only for old cores lacking LLOCK/SCOND, which by defintion @@ -60,23 +62,47 @@ SYSCALL_DEFINE3(arc_usr_cmpxchg, int *, uaddr, int, expected, int, new) /* Z indicates to userspace if operation succeded */ regs->status32 &= ~STATUS_Z_MASK; - if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int))) - return -EFAULT; + ret = access_ok(VERIFY_WRITE, uaddr, sizeof(*uaddr)); + if (!ret) + goto fail; +again: preempt_disable(); - if (__get_user(uval, uaddr)) - goto done; + ret = __get_user(val, uaddr); + if (ret) + goto fault; - if (uval == expected) { - if (!__put_user(new, uaddr)) - regs->status32 |= STATUS_Z_MASK; - } + if (val != expected) + goto out; + + ret = __put_user(new, uaddr); + if (ret) + goto fault; + + regs->status32 |= STATUS_Z_MASK; + +out: + preempt_enable(); + return val; -done: +fault: preempt_enable(); - return uval; + if (unlikely(ret != -EFAULT)) + goto fail; + + down_read(¤t->mm->mmap_sem); + ret = fixup_user_fault(current, current->mm, uaddr, FAULT_FLAG_WRITE, + NULL); + up_read(¤t->mm->mmap_sem); + + if (likely(!ret)) + goto again; + +fail: + force_sig(SIGSEGV, current); + return ret; } #ifdef CONFIG_ISA_ARCV2