From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754346AbbEMJwz (ORCPT ); Wed, 13 May 2015 05:52:55 -0400 Received: from mail.skyhub.de ([78.46.96.112]:60656 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753450AbbEMJwx (ORCPT ); Wed, 13 May 2015 05:52:53 -0400 Date: Wed, 13 May 2015 11:52:48 +0200 From: Borislav Petkov To: Linus Torvalds Cc: Ingo Molnar , "H. Peter Anvin" , Thomas Gleixner , Andy Lutomirski , Denys Vlasenko , lkml Subject: Re: [RFC PATCH] Drop some asm from copy_user_64.S Message-ID: <20150513095248.GD1517@pd.tnic> References: <20150512205750.GJ3497@pd.tnic> <20150512215320.GK3497@pd.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150512215320.GK3497@pd.tnic> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 12, 2015 at 11:53:20PM +0200, Borislav Petkov wrote: > > That said, I think you should uninline those things, and move them > > from a header file to a C file (arch/x86/lib/uaccess.c?). It is starting to look better wrt size: x86_64_defconfig: text data bss dec hex filename before: 12375798 1812800 1085440 15274038 e91036 vmlinux after: 12269658 1812800 1085440 15167898 e7719a vmlinux Now we CALL _copy_*_user which does CALL the optimal alternative version. Advantage is that we're saving some space and alternatives application for copy_user* is being done in less places, i.e. arch/x86/lib/uaccess_64.c. If I move all copy_user_generic() callers there, it would be the only compilation unit where the alternatives will be done. The disadvantage is that we have CALL after CALL and I wanted to have a single CALL directly to the optimal copy_user function. That'll cost us space, though, and more alternatives sites to patch during boot... Thoughts? Here's the first step only: --- arch/x86/include/asm/uaccess.h | 7 ++----- arch/x86/include/asm/uaccess_64.h | 44 ++++----------------------------------- arch/x86/lib/Makefile | 2 +- arch/x86/lib/uaccess_64.c | 42 +++++++++++++++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 46 deletions(-) create mode 100644 arch/x86/lib/uaccess_64.c diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h index 098f3fd5cc75..bdd5234fe9a3 100644 --- a/arch/x86/include/asm/uaccess.h +++ b/arch/x86/include/asm/uaccess.h @@ -642,11 +642,8 @@ extern struct movsl_mask { # include #endif -extern __always_inline __must_check -int _copy_from_user(void *dst, const void __user *src, unsigned size); - -extern __always_inline __must_check -int _copy_to_user(void __user *dst, const void *src, unsigned size); +extern __must_check int _copy_from_user(void *dst, const void __user *src, unsigned size); +extern __must_check int _copy_to_user(void __user *dst, const void *src, unsigned size); #ifdef CONFIG_DEBUG_STRICT_USER_COPY_CHECKS # define copy_user_diag __compiletime_error diff --git a/arch/x86/include/asm/uaccess_64.h b/arch/x86/include/asm/uaccess_64.h index 1aebc658acf9..440ba6b91c86 100644 --- a/arch/x86/include/asm/uaccess_64.h +++ b/arch/x86/include/asm/uaccess_64.h @@ -23,31 +23,11 @@ copy_user_generic_string(void *to, const void *from, unsigned len); __must_check unsigned long copy_user_generic_unrolled(void *to, const void *from, unsigned len); -static __always_inline __must_check unsigned long -copy_user_generic(void *to, const void *from, unsigned len) -{ - unsigned ret; - - /* - * If CPU has ERMS feature, use copy_user_enhanced_fast_string. - * Otherwise, if CPU has rep_good feature, use copy_user_generic_string. - * Otherwise, use copy_user_generic_unrolled. - */ - alternative_call_2(copy_user_generic_unrolled, - copy_user_generic_string, - X86_FEATURE_REP_GOOD, - copy_user_enhanced_fast_string, - X86_FEATURE_ERMS, - ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from), - "=d" (len)), - "1" (to), "2" (from), "3" (len) - : "memory", "rcx", "r8", "r9", "r10", "r11"); - return ret; -} - __must_check unsigned long copy_in_user(void __user *to, const void __user *from, unsigned len); +extern __must_check unsigned long copy_user_generic(void *to, const void *from, unsigned len); + static __always_inline __must_check int __copy_from_user_nocheck(void *dst, const void __user *src, unsigned size) { @@ -98,16 +78,7 @@ int __copy_from_user(void *dst, const void __user *src, unsigned size) return __copy_from_user_nocheck(dst, src, size); } -static __always_inline __must_check -int _copy_from_user(void *dst, const void __user *src, unsigned size) -{ - if (!access_ok(VERIFY_READ, src, size)) { - memset(dst, 0, size); - return 0; - } - - return copy_user_generic(dst, src, size); -} +extern __must_check int _copy_from_user(void *dst, const void __user *src, unsigned size); static __always_inline __must_check int __copy_to_user_nocheck(void __user *dst, const void *src, unsigned size) @@ -159,14 +130,7 @@ int __copy_to_user(void __user *dst, const void *src, unsigned size) return __copy_to_user_nocheck(dst, src, size); } -static __always_inline __must_check -int _copy_to_user(void __user *dst, const void *src, unsigned size) -{ - if (!access_ok(VERIFY_WRITE, dst, size)) - return size; - - return copy_user_generic(dst, src, size); -} +__must_check int _copy_to_user(void __user *dst, const void *src, unsigned size); static __always_inline __must_check int __copy_in_user(void __user *dst, const void __user *src, unsigned size) diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index 982989d282ff..1885cc5eee32 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -40,6 +40,6 @@ else lib-y += csum-partial_64.o csum-copy_64.o csum-wrappers_64.o lib-y += clear_page_64.o copy_page_64.o lib-y += memmove_64.o memset_64.o - lib-y += copy_user_64.o + lib-y += copy_user_64.o uaccess_64.o lib-y += cmpxchg16b_emu.o endif diff --git a/arch/x86/lib/uaccess_64.c b/arch/x86/lib/uaccess_64.c new file mode 100644 index 000000000000..6cd15d874ab4 --- /dev/null +++ b/arch/x86/lib/uaccess_64.c @@ -0,0 +1,42 @@ +#include + +__always_inline __must_check unsigned long +copy_user_generic(void *to, const void *from, unsigned len) +{ + unsigned ret; + + /* + * If CPU has ERMS feature, use copy_user_enhanced_fast_string. + * Otherwise, if CPU has rep_good feature, use copy_user_generic_string. + * Otherwise, use copy_user_generic_unrolled. + */ + alternative_call_2(copy_user_generic_unrolled, + copy_user_generic_string, + X86_FEATURE_REP_GOOD, + copy_user_enhanced_fast_string, + X86_FEATURE_ERMS, + ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from), + "=d" (len)), + "1" (to), "2" (from), "3" (len) + : "memory", "rcx", "r8", "r9", "r10", "r11"); + return ret; +} +EXPORT_SYMBOL_GPL(copy_user_generic); + +__must_check int _copy_from_user(void *dst, const void __user *src, unsigned size) +{ + if (!access_ok(VERIFY_READ, src, size)) { + memset(dst, 0, size); + return 0; + } + + return copy_user_generic(dst, src, size); +} + +__must_check int _copy_to_user(void __user *dst, const void *src, unsigned size) +{ + if (!access_ok(VERIFY_WRITE, dst, size)) + return size; + + return copy_user_generic(dst, src, size); +} -- 2.3.5 -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --