From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id B653AC433EF for ; Tue, 15 Feb 2022 11:08:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Xi0AY+inwtUeNI3lnIl39TLHr1MhZFrfT2wK1J6x6fw=; b=2bbBb1bKL1rpDY iY2B2FSXocjaK0C1u3EO7J2+RBQWXRoG28TQ19CXg0iubVx0tMxV5WZF60zEKwsitla0fVLeMCKGp P4ZKYxqQhuvAAOuKBPB3BNSqbzCHR1u1lZXgKtZ5vzaKDMx0ls2iHZVY81EdyCTF/mfueGfG4BFK3 xNmycc9dk+hv8nB1uKUxlSUjZaAuzNyIZBJkyxEyS4wLI28HrxvAUz3q5T3C7MaqymJiPgRN86pkV yZIPjyrs3vMOd6OTqFT5/MgGIHd+hqgJMSQPAKHuzTAB5LUlTNy23qI0U+xgaYQt2Gx/V2hXl5IZV qHE3HQ4aSKhlSN1BJk7w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nJvg8-002TF0-L9; Tue, 15 Feb 2022 11:07:28 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nJvg4-002TDP-0y; Tue, 15 Feb 2022 11:07:25 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D50F51424; Tue, 15 Feb 2022 03:07:22 -0800 (PST) Received: from FVFF77S0Q05N (unknown [10.57.89.144]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3D4923F718; Tue, 15 Feb 2022 03:07:16 -0800 (PST) Date: Tue, 15 Feb 2022 11:07:12 +0000 From: Mark Rutland To: Arnd Bergmann Cc: Linus Torvalds , Christoph Hellwig , linux-arch@vger.kernel.org, linux-mm@kvack.org, linux-api@vger.kernel.org, arnd@arndb.de, linux-kernel@vger.kernel.org, linux@armlinux.org.uk, will@kernel.org, guoren@kernel.org, bcain@codeaurora.org, geert@linux-m68k.org, monstr@monstr.eu, tsbogend@alpha.franken.de, nickhu@andestech.com, green.hu@gmail.com, dinguyen@kernel.org, shorne@gmail.com, deller@gmx.de, mpe@ellerman.id.au, peterz@infradead.org, mingo@redhat.com, hca@linux.ibm.com, dalias@libc.org, davem@davemloft.net, richard@nod.at, x86@kernel.org, jcmvbkbc@gmail.com, ebiederm@xmission.com, akpm@linux-foundation.org, ardb@kernel.org, linux-alpha@vger.kernel.org, linux-snps-arc@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-csky@vger.kernel.org, linux-hexagon@vger.kernel.org, linux-ia64@vger.kernel.org, linux-m68k@lists.linux-m68k.org, linux-mips@vger.kernel.org, openrisc@lists.librecores.org, linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, linux-sh@vger.kernel.org, sparclinux@vger.kernel.org, linux-um@lists.infradead.org, linux-xtensa@linux-xtensa.org Subject: Re: [PATCH 08/14] arm64: simplify access_ok() Message-ID: References: <20220214163452.1568807-1-arnd@kernel.org> <20220214163452.1568807-9-arnd@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220214163452.1568807-9-arnd@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220215_030724_188972_E8FEB4DF X-CRM114-Status: GOOD ( 33.49 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Feb 14, 2022 at 05:34:46PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann > > arm64 has an inline asm implementation of access_ok() that is derived from > the 32-bit arm version and optimized for the case that both the limit and > the size are variable. With set_fs() gone, the limit is always constant, > and the size usually is as well, so just using the default implementation > reduces the check into a comparison against a constant that can be > scheduled by the compiler. > > On a defconfig build, this saves over 28KB of .text. > > Signed-off-by: Arnd Bergmann I had a play around with this and a number of alternative options that had previously been discussed (e.g. using uint128_t for the check to allow the compiler to use the carry flag), and: * Any sequences which we significantly simpler involved an ABI change (e.g. not checking tags for tasks not using the relaxed tag ABI), or didn't interact well with the uaccess pointer masking we do for speculation hardening. * For all constant-size cases, this was joint-best for codegen. * For variable-size cases the difference between options (which did not change ABI or break pointer masking) fell in the noise and really depended on what you were optimizing for. This patch itself is clear, I believe the logic is sound and does not result in a behavioural change, so for this as-is: Acked-by: Mark Rutland As on other replies, I think that if we want to make further changes to this, we should do that as follow-ups, since there are a number of subtleties in this area w.r.t. tag management and speculation with potential ABI implications. Thanks, Mark. > --- > arch/arm64/include/asm/uaccess.h | 28 +++++----------------------- > 1 file changed, 5 insertions(+), 23 deletions(-) > > diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h > index 357f7bd9c981..e8dce0cc5eaa 100644 > --- a/arch/arm64/include/asm/uaccess.h > +++ b/arch/arm64/include/asm/uaccess.h > @@ -26,6 +26,8 @@ > #include > #include > > +static inline int __access_ok(const void __user *ptr, unsigned long size); > + > /* > * Test whether a block of memory is a valid user space address. > * Returns 1 if the range is valid, 0 otherwise. > @@ -33,10 +35,8 @@ > * This is equivalent to the following test: > * (u65)addr + (u65)size <= (u65)TASK_SIZE_MAX > */ > -static inline unsigned long __access_ok(const void __user *addr, unsigned long size) > +static inline int access_ok(const void __user *addr, unsigned long size) > { > - unsigned long ret, limit = TASK_SIZE_MAX - 1; > - > /* > * Asynchronous I/O running in a kernel thread does not have the > * TIF_TAGGED_ADDR flag of the process owning the mm, so always untag > @@ -46,27 +46,9 @@ static inline unsigned long __access_ok(const void __user *addr, unsigned long s > (current->flags & PF_KTHREAD || test_thread_flag(TIF_TAGGED_ADDR))) > addr = untagged_addr(addr); > > - __chk_user_ptr(addr); > - asm volatile( > - // A + B <= C + 1 for all A,B,C, in four easy steps: > - // 1: X = A + B; X' = X % 2^64 > - " adds %0, %3, %2\n" > - // 2: Set C = 0 if X > 2^64, to guarantee X' > C in step 4 > - " csel %1, xzr, %1, hi\n" > - // 3: Set X' = ~0 if X >= 2^64. For X == 2^64, this decrements X' > - // to compensate for the carry flag being set in step 4. For > - // X > 2^64, X' merely has to remain nonzero, which it does. > - " csinv %0, %0, xzr, cc\n" > - // 4: For X < 2^64, this gives us X' - C - 1 <= 0, where the -1 > - // comes from the carry in being clear. Otherwise, we are > - // testing X' - C == 0, subject to the previous adjustments. > - " sbcs xzr, %0, %1\n" > - " cset %0, ls\n" > - : "=&r" (ret), "+r" (limit) : "Ir" (size), "0" (addr) : "cc"); > - > - return ret; > + return likely(__access_ok(addr, size)); > } > -#define __access_ok __access_ok > +#define access_ok access_ok > > #include > > -- > 2.29.2 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel