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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 541F2C433F5 for ; Tue, 15 Feb 2022 11:07:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 928BE6B0078; Tue, 15 Feb 2022 06:07:24 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8D73A6B007B; Tue, 15 Feb 2022 06:07:24 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 79F056B007D; Tue, 15 Feb 2022 06:07:24 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0203.hostedemail.com [216.40.44.203]) by kanga.kvack.org (Postfix) with ESMTP id 69E356B0078 for ; Tue, 15 Feb 2022 06:07:24 -0500 (EST) Received: from smtpin28.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 2408E180AD837 for ; Tue, 15 Feb 2022 11:07:24 +0000 (UTC) X-FDA: 79144738008.28.976CD92 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf12.hostedemail.com (Postfix) with ESMTP id 9CF1640009 for ; Tue, 15 Feb 2022 11:07:23 +0000 (UTC) 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-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220214163452.1568807-9-arnd@kernel.org> X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 9CF1640009 X-Stat-Signature: gh6nm74d6t66j3iieghfk4me47nz4u6p X-Rspam-User: Authentication-Results: imf12.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf12.hostedemail.com: domain of mark.rutland@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=mark.rutland@arm.com X-HE-Tag: 1644923243-707443 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: 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 >