[PATCH 1/2] x86: math-emu: check __copy_from_user result

[PATCH 1/2] x86: math-emu: check __copy_from_user result

[Arnd Bergmann]

The new __must_check annotation on __copy_from_user successfully
identified some code that has lacked the check since at least
linux-2.1.73:

arch/x86/math-emu/reg_ld_str.c:88:2: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
        __copy_from_user(sti_ptr, s, 10);
        ^~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~
arch/x86/math-emu/reg_ld_str.c:1129:2: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
        __copy_from_user(register_base + offset, s, other);
        ^~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/x86/math-emu/reg_ld_str.c:1131:3: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
                __copy_from_user(register_base, s + other, offset);
                ^~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In addition, the get_user/put_user helpers do not enforce a return value
check, but actually still require one. These have been missing
for even longer.

Change the internal wrappers around get_user/put_user to force
a signal and add a corresponding wrapper around __copy_from_user
to check all such cases.

Fixes: 257e458057e5 ("Import 2.1.73")
Fixes: 9dd819a15162 ("uaccess: add missing __must_check attributes")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/math-emu/fpu_system.h | 6 ++++--
 arch/x86/math-emu/reg_ld_str.c | 6 +++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/x86/math-emu/fpu_system.h b/arch/x86/math-emu/fpu_system.h
index f98a0c956764..9b41391867dc 100644
--- a/arch/x86/math-emu/fpu_system.h
+++ b/arch/x86/math-emu/fpu_system.h
@@ -107,6 +107,8 @@ static inline bool seg_writable(struct desc_struct *d)
 #define FPU_access_ok(y,z)	if ( !access_ok(y,z) ) \
 				math_abort(FPU_info,SIGSEGV)
 #define FPU_abort		math_abort(FPU_info, SIGSEGV)
+#define FPU_copy_from_user(to, from, n)	\
+		do { if (copy_from_user(to, from, n)) FPU_abort; } while (0)
 
 #undef FPU_IGNORE_CODE_SEGV
 #ifdef FPU_IGNORE_CODE_SEGV
@@ -122,7 +124,7 @@ static inline bool seg_writable(struct desc_struct *d)
 #define	FPU_code_access_ok(z) FPU_access_ok((void __user *)FPU_EIP,z)
 #endif
 
-#define FPU_get_user(x,y)       get_user((x),(y))
-#define FPU_put_user(x,y)       put_user((x),(y))
+#define FPU_get_user(x,y) do { if (get_user((x),(y))) FPU_abort; } while (0)
+#define FPU_put_user(x,y) do { if (put_user((x),(y))) FPU_abort; } while (0)
 
 #endif
diff --git a/arch/x86/math-emu/reg_ld_str.c b/arch/x86/math-emu/reg_ld_str.c
index f3779743d15e..fe6246ff9887 100644
--- a/arch/x86/math-emu/reg_ld_str.c
+++ b/arch/x86/math-emu/reg_ld_str.c
@@ -85,7 +85,7 @@ int FPU_load_extended(long double __user *s, int stnr)
 
 	RE_ENTRANT_CHECK_OFF;
 	FPU_access_ok(s, 10);
-	__copy_from_user(sti_ptr, s, 10);
+	FPU_copy_from_user(sti_ptr, s, 10);
 	RE_ENTRANT_CHECK_ON;
 
 	return FPU_tagof(sti_ptr);
@@ -1126,9 +1126,9 @@ void frstor(fpu_addr_modes addr_modes, u_char __user *data_address)
 	/* Copy all registers in stack order. */
 	RE_ENTRANT_CHECK_OFF;
 	FPU_access_ok(s, 80);
-	__copy_from_user(register_base + offset, s, other);
+	FPU_copy_from_user(register_base + offset, s, other);
 	if (offset)
-		__copy_from_user(register_base, s + other, offset);
+		FPU_copy_from_user(register_base, s + other, offset);
 	RE_ENTRANT_CHECK_ON;
 
 	for (i = 0; i < 8; i++) {
-- 
2.20.0

[PATCH 2/2] x86: math-emu: limit MATH_EMULATION to 486SX compatibles

[Arnd Bergmann]

The fpu emulation code is old and fragile in places, try to limit its
use to builds for CPUs that actually use it. As far as I can tell,
this is only true for i486sx compatibles, including the Cyrix 486SLC,
AMD Am486SX and ÉLAN SC410, UMC U5S amd DM&P VortexSX86, all of which
were relatively short-lived and got replaced with i486DX compatible
processors soon after introduction, though the some of the embedded
versions remained available much longer.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/Kconfig              |  2 +-
 arch/x86/Kconfig.cpu          | 25 ++++++++++++++++---------
 arch/x86/Makefile_32.cpu      |  1 +
 arch/x86/include/asm/module.h |  2 ++
 4 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index f4d9d1e55e5c..77b02387bd0c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1751,7 +1751,7 @@ config X86_RESERVE_LOW
 config MATH_EMULATION
 	bool
 	depends on MODIFY_LDT_SYSCALL
-	prompt "Math emulation" if X86_32
+	prompt "Math emulation" if X86_32 && (M486SX || MELAN)
 	---help---
 	  Linux can emulate a math coprocessor (used for floating point
 	  operations) if you don't have one. 486DX and Pentium processors have
diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu
index 228705a1232a..5f7bff9885a1 100644
--- a/arch/x86/Kconfig.cpu
+++ b/arch/x86/Kconfig.cpu
@@ -50,12 +50,19 @@ choice
 	  See each option's help text for additional details. If you don't know
 	  what to do, choose "486".
 
+config M486SX
+	bool "486SX"
+	depends on X86_32
+	---help---
+	  Select this for an 486-class CPU without an FPU such as
+	  AMD/Cyrix/IBM/Intel SL/SLC/SLC2/SLC3/SX/SX2 and UMC U5S.
+
 config M486
-	bool "486"
+	bool "486DX"
 	depends on X86_32
 	---help---
 	  Select this for an 486-class CPU such as AMD/Cyrix/IBM/Intel
-	  486DX/DX2/DX4 or SL/SLC/SLC2/SLC3/SX/SX2 and UMC U5D or U5S.
+	  486DX/DX2/DX4 and UMC U5D.
 
 config M586
 	bool "586/K5/5x86/6x86/6x86MX"
@@ -313,20 +320,20 @@ config X86_L1_CACHE_SHIFT
 	int
 	default "7" if MPENTIUM4 || MPSC
 	default "6" if MK7 || MK8 || MPENTIUMM || MCORE2 || MATOM || MVIAC7 || X86_GENERIC || GENERIC_CPU
-	default "4" if MELAN || M486 || MGEODEGX1
+	default "4" if MELAN || M486SX || M486 || MGEODEGX1
 	default "5" if MWINCHIP3D || MWINCHIPC6 || MCRUSOE || MEFFICEON || MCYRIXIII || MK6 || MPENTIUMIII || MPENTIUMII || M686 || M586MMX || M586TSC || M586 || MVIAC3_2 || MGEODE_LX
 
 config X86_F00F_BUG
 	def_bool y
-	depends on M586MMX || M586TSC || M586 || M486
+	depends on M586MMX || M586TSC || M586 || M486SX || M486
 
 config X86_INVD_BUG
 	def_bool y
-	depends on M486
+	depends on M486SX || M486
 
 config X86_ALIGNMENT_16
 	def_bool y
-	depends on MWINCHIP3D || MWINCHIPC6 || MCYRIXIII || MELAN || MK6 || M586MMX || M586TSC || M586 || M486 || MVIAC3_2 || MGEODEGX1
+	depends on MWINCHIP3D || MWINCHIPC6 || MCYRIXIII || MELAN || MK6 || M586MMX || M586TSC || M586 || M486SX || M486 || MVIAC3_2 || MGEODEGX1
 
 config X86_INTEL_USERCOPY
 	def_bool y
@@ -379,7 +386,7 @@ config X86_MINIMUM_CPU_FAMILY
 
 config X86_DEBUGCTLMSR
 	def_bool y
-	depends on !(MK6 || MWINCHIPC6 || MWINCHIP3D || MCYRIXIII || M586MMX || M586TSC || M586 || M486) && !UML
+	depends on !(MK6 || MWINCHIPC6 || MWINCHIP3D || MCYRIXIII || M586MMX || M586TSC || M586 || M486SX || M486) && !UML
 
 menuconfig PROCESSOR_SELECT
 	bool "Supported processor vendors" if EXPERT
@@ -403,7 +410,7 @@ config CPU_SUP_INTEL
 config CPU_SUP_CYRIX_32
 	default y
 	bool "Support Cyrix processors" if PROCESSOR_SELECT
-	depends on M486 || M586 || M586TSC || M586MMX || (EXPERT && !64BIT)
+	depends on M486SX || M486 || M586 || M586TSC || M586MMX || (EXPERT && !64BIT)
 	---help---
 	  This enables detection, tunings and quirks for Cyrix processors
 
@@ -471,7 +478,7 @@ config CPU_SUP_TRANSMETA_32
 config CPU_SUP_UMC_32
 	default y
 	bool "Support UMC processors" if PROCESSOR_SELECT
-	depends on M486 || (EXPERT && !64BIT)
+	depends on M486SX || M486 || (EXPERT && !64BIT)
 	---help---
 	  This enables detection, tunings and quirks for UMC processors
 
diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index 1f5faf8606b4..cd3056759880 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -10,6 +10,7 @@ else
 tune		= $(call cc-option,-mcpu=$(1),$(2))
 endif
 
+cflags-$(CONFIG_M486SX)		+= -march=i486
 cflags-$(CONFIG_M486)		+= -march=i486
 cflags-$(CONFIG_M586)		+= -march=i586
 cflags-$(CONFIG_M586TSC)	+= -march=i586
diff --git a/arch/x86/include/asm/module.h b/arch/x86/include/asm/module.h
index 7948a17febb4..c215d2762488 100644
--- a/arch/x86/include/asm/module.h
+++ b/arch/x86/include/asm/module.h
@@ -15,6 +15,8 @@ struct mod_arch_specific {
 
 #ifdef CONFIG_X86_64
 /* X86_64 does not define MODULE_PROC_FAMILY */
+#elif defined CONFIG_M486SX
+#define MODULE_PROC_FAMILY "486SX "
 #elif defined CONFIG_M486
 #define MODULE_PROC_FAMILY "486 "
 #elif defined CONFIG_M586
-- 
2.20.0

Re: [PATCH 2/2] x86: math-emu: limit MATH_EMULATION to 486SX compatibles

[Kees Cook]

On Tue, Oct 01, 2019 at 04:23:35PM +0200, Arnd Bergmann wrote:
> The fpu emulation code is old and fragile in places, try to limit its
> use to builds for CPUs that actually use it. As far as I can tell,
> this is only true for i486sx compatibles, including the Cyrix 486SLC,
> AMD Am486SX and ÉLAN SC410, UMC U5S amd DM&P VortexSX86, all of which
> were relatively short-lived and got replaced with i486DX compatible
> processors soon after introduction, though the some of the embedded
> versions remained available much longer.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Nice, I like carving out CONFIG space for 486SX; this makes sense to me.

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/x86/Kconfig              |  2 +-
>  arch/x86/Kconfig.cpu          | 25 ++++++++++++++++---------
>  arch/x86/Makefile_32.cpu      |  1 +
>  arch/x86/include/asm/module.h |  2 ++
>  4 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index f4d9d1e55e5c..77b02387bd0c 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1751,7 +1751,7 @@ config X86_RESERVE_LOW
>  config MATH_EMULATION
>  	bool
>  	depends on MODIFY_LDT_SYSCALL
> -	prompt "Math emulation" if X86_32
> +	prompt "Math emulation" if X86_32 && (M486SX || MELAN)
>  	---help---
>  	  Linux can emulate a math coprocessor (used for floating point
>  	  operations) if you don't have one. 486DX and Pentium processors have
> diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu
> index 228705a1232a..5f7bff9885a1 100644
> --- a/arch/x86/Kconfig.cpu
> +++ b/arch/x86/Kconfig.cpu
> @@ -50,12 +50,19 @@ choice
>  	  See each option's help text for additional details. If you don't know
>  	  what to do, choose "486".
>  
> +config M486SX
> +	bool "486SX"
> +	depends on X86_32
> +	---help---
> +	  Select this for an 486-class CPU without an FPU such as
> +	  AMD/Cyrix/IBM/Intel SL/SLC/SLC2/SLC3/SX/SX2 and UMC U5S.
> +
>  config M486
> -	bool "486"
> +	bool "486DX"
>  	depends on X86_32
>  	---help---
>  	  Select this for an 486-class CPU such as AMD/Cyrix/IBM/Intel
> -	  486DX/DX2/DX4 or SL/SLC/SLC2/SLC3/SX/SX2 and UMC U5D or U5S.
> +	  486DX/DX2/DX4 and UMC U5D.
>  
>  config M586
>  	bool "586/K5/5x86/6x86/6x86MX"
> @@ -313,20 +320,20 @@ config X86_L1_CACHE_SHIFT
>  	int
>  	default "7" if MPENTIUM4 || MPSC
>  	default "6" if MK7 || MK8 || MPENTIUMM || MCORE2 || MATOM || MVIAC7 || X86_GENERIC || GENERIC_CPU
> -	default "4" if MELAN || M486 || MGEODEGX1
> +	default "4" if MELAN || M486SX || M486 || MGEODEGX1
>  	default "5" if MWINCHIP3D || MWINCHIPC6 || MCRUSOE || MEFFICEON || MCYRIXIII || MK6 || MPENTIUMIII || MPENTIUMII || M686 || M586MMX || M586TSC || M586 || MVIAC3_2 || MGEODE_LX
>  
>  config X86_F00F_BUG
>  	def_bool y
> -	depends on M586MMX || M586TSC || M586 || M486
> +	depends on M586MMX || M586TSC || M586 || M486SX || M486
>  
>  config X86_INVD_BUG
>  	def_bool y
> -	depends on M486
> +	depends on M486SX || M486
>  
>  config X86_ALIGNMENT_16
>  	def_bool y
> -	depends on MWINCHIP3D || MWINCHIPC6 || MCYRIXIII || MELAN || MK6 || M586MMX || M586TSC || M586 || M486 || MVIAC3_2 || MGEODEGX1
> +	depends on MWINCHIP3D || MWINCHIPC6 || MCYRIXIII || MELAN || MK6 || M586MMX || M586TSC || M586 || M486SX || M486 || MVIAC3_2 || MGEODEGX1
>  
>  config X86_INTEL_USERCOPY
>  	def_bool y
> @@ -379,7 +386,7 @@ config X86_MINIMUM_CPU_FAMILY
>  
>  config X86_DEBUGCTLMSR
>  	def_bool y
> -	depends on !(MK6 || MWINCHIPC6 || MWINCHIP3D || MCYRIXIII || M586MMX || M586TSC || M586 || M486) && !UML
> +	depends on !(MK6 || MWINCHIPC6 || MWINCHIP3D || MCYRIXIII || M586MMX || M586TSC || M586 || M486SX || M486) && !UML
>  
>  menuconfig PROCESSOR_SELECT
>  	bool "Supported processor vendors" if EXPERT
> @@ -403,7 +410,7 @@ config CPU_SUP_INTEL
>  config CPU_SUP_CYRIX_32
>  	default y
>  	bool "Support Cyrix processors" if PROCESSOR_SELECT
> -	depends on M486 || M586 || M586TSC || M586MMX || (EXPERT && !64BIT)
> +	depends on M486SX || M486 || M586 || M586TSC || M586MMX || (EXPERT && !64BIT)
>  	---help---
>  	  This enables detection, tunings and quirks for Cyrix processors
>  
> @@ -471,7 +478,7 @@ config CPU_SUP_TRANSMETA_32
>  config CPU_SUP_UMC_32
>  	default y
>  	bool "Support UMC processors" if PROCESSOR_SELECT
> -	depends on M486 || (EXPERT && !64BIT)
> +	depends on M486SX || M486 || (EXPERT && !64BIT)
>  	---help---
>  	  This enables detection, tunings and quirks for UMC processors
>  
> diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
> index 1f5faf8606b4..cd3056759880 100644
> --- a/arch/x86/Makefile_32.cpu
> +++ b/arch/x86/Makefile_32.cpu
> @@ -10,6 +10,7 @@ else
>  tune		= $(call cc-option,-mcpu=$(1),$(2))
>  endif
>  
> +cflags-$(CONFIG_M486SX)		+= -march=i486
>  cflags-$(CONFIG_M486)		+= -march=i486
>  cflags-$(CONFIG_M586)		+= -march=i586
>  cflags-$(CONFIG_M586TSC)	+= -march=i586
> diff --git a/arch/x86/include/asm/module.h b/arch/x86/include/asm/module.h
> index 7948a17febb4..c215d2762488 100644
> --- a/arch/x86/include/asm/module.h
> +++ b/arch/x86/include/asm/module.h
> @@ -15,6 +15,8 @@ struct mod_arch_specific {
>  
>  #ifdef CONFIG_X86_64
>  /* X86_64 does not define MODULE_PROC_FAMILY */
> +#elif defined CONFIG_M486SX
> +#define MODULE_PROC_FAMILY "486SX "
>  #elif defined CONFIG_M486
>  #define MODULE_PROC_FAMILY "486 "
>  #elif defined CONFIG_M586
> -- 
> 2.20.0
> 

-- 
Kees Cook

Re: [PATCH 1/2] x86: math-emu: check __copy_from_user result

[Kees Cook]

On Tue, Oct 01, 2019 at 04:23:34PM +0200, Arnd Bergmann wrote:
> The new __must_check annotation on __copy_from_user successfully
> identified some code that has lacked the check since at least
> linux-2.1.73:
> 
> arch/x86/math-emu/reg_ld_str.c:88:2: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
>         __copy_from_user(sti_ptr, s, 10);
>         ^~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~
> arch/x86/math-emu/reg_ld_str.c:1129:2: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
>         __copy_from_user(register_base + offset, s, other);
>         ^~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> arch/x86/math-emu/reg_ld_str.c:1131:3: error: ignoring return value of function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
>                 __copy_from_user(register_base, s + other, offset);
>                 ^~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> In addition, the get_user/put_user helpers do not enforce a return value
> check, but actually still require one. These have been missing
> for even longer.
> 
> Change the internal wrappers around get_user/put_user to force
> a signal and add a corresponding wrapper around __copy_from_user
> to check all such cases.
> 
> Fixes: 257e458057e5 ("Import 2.1.73")
> Fixes: 9dd819a15162 ("uaccess: add missing __must_check attributes")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Kees Cook <keescook@chromium.org>

Notes below...

> ---
>  arch/x86/math-emu/fpu_system.h | 6 ++++--
>  arch/x86/math-emu/reg_ld_str.c | 6 +++---
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/math-emu/fpu_system.h b/arch/x86/math-emu/fpu_system.h
> index f98a0c956764..9b41391867dc 100644
> --- a/arch/x86/math-emu/fpu_system.h
> +++ b/arch/x86/math-emu/fpu_system.h
> @@ -107,6 +107,8 @@ static inline bool seg_writable(struct desc_struct *d)
>  #define FPU_access_ok(y,z)	if ( !access_ok(y,z) ) \
>  				math_abort(FPU_info,SIGSEGV)
>  #define FPU_abort		math_abort(FPU_info, SIGSEGV)
> +#define FPU_copy_from_user(to, from, n)	\
> +		do { if (copy_from_user(to, from, n)) FPU_abort; } while (0)
>  
>  #undef FPU_IGNORE_CODE_SEGV
>  #ifdef FPU_IGNORE_CODE_SEGV
> @@ -122,7 +124,7 @@ static inline bool seg_writable(struct desc_struct *d)
>  #define	FPU_code_access_ok(z) FPU_access_ok((void __user *)FPU_EIP,z)
>  #endif
>  
> -#define FPU_get_user(x,y)       get_user((x),(y))
> -#define FPU_put_user(x,y)       put_user((x),(y))
> +#define FPU_get_user(x,y) do { if (get_user((x),(y))) FPU_abort; } while (0)
> +#define FPU_put_user(x,y) do { if (put_user((x),(y))) FPU_abort; } while (0)
>  
>  #endif
> diff --git a/arch/x86/math-emu/reg_ld_str.c b/arch/x86/math-emu/reg_ld_str.c
> index f3779743d15e..fe6246ff9887 100644
> --- a/arch/x86/math-emu/reg_ld_str.c
> +++ b/arch/x86/math-emu/reg_ld_str.c
> @@ -85,7 +85,7 @@ int FPU_load_extended(long double __user *s, int stnr)
>  
>  	RE_ENTRANT_CHECK_OFF;
>  	FPU_access_ok(s, 10);
> -	__copy_from_user(sti_ptr, s, 10);
> +	FPU_copy_from_user(sti_ptr, s, 10);

These access_ok() checks seem redundant everywhere in this file (after
your switch from __copy* to copy*. I mean, I guess, just leave them, but
*shrug*

-Kees

>  	RE_ENTRANT_CHECK_ON;
>  
>  	return FPU_tagof(sti_ptr);
> @@ -1126,9 +1126,9 @@ void frstor(fpu_addr_modes addr_modes, u_char __user *data_address)
>  	/* Copy all registers in stack order. */
>  	RE_ENTRANT_CHECK_OFF;
>  	FPU_access_ok(s, 80);
> -	__copy_from_user(register_base + offset, s, other);
> +	FPU_copy_from_user(register_base + offset, s, other);
>  	if (offset)
> -		__copy_from_user(register_base, s + other, offset);
> +		FPU_copy_from_user(register_base, s + other, offset);
>  	RE_ENTRANT_CHECK_ON;
>  
>  	for (i = 0; i < 8; i++) {
> -- 
> 2.20.0
> 

-- 
Kees Cook

Re: [PATCH 1/2] x86: math-emu: check __copy_from_user result

[Arnd Bergmann]

On Wed, Oct 2, 2019 at 1:39 AM Kees Cook <keescook@chromium.org> wrote:

> > diff --git a/arch/x86/math-emu/reg_ld_str.c b/arch/x86/math-emu/reg_ld_str.c
> > index f3779743d15e..fe6246ff9887 100644
> > --- a/arch/x86/math-emu/reg_ld_str.c
> > +++ b/arch/x86/math-emu/reg_ld_str.c
> > @@ -85,7 +85,7 @@ int FPU_load_extended(long double __user *s, int stnr)
> >
> >       RE_ENTRANT_CHECK_OFF;
> >       FPU_access_ok(s, 10);
> > -     __copy_from_user(sti_ptr, s, 10);
> > +     FPU_copy_from_user(sti_ptr, s, 10);
>
> These access_ok() checks seem redundant everywhere in this file (after
> your switch from __copy* to copy*. I mean, I guess, just leave them, but
> *shrug*

There have always been duplicate/inconsistent for the get_user/put_user
case. I considered cleaning it all up but then decided to touch it as little
as possible.

       Arnd

Re: [PATCH 1/2] x86: math-emu: check __copy_from_user result

[Kees Cook]

On Wed, Oct 02, 2019 at 09:11:23AM +0200, Arnd Bergmann wrote:
> On Wed, Oct 2, 2019 at 1:39 AM Kees Cook <keescook@chromium.org> wrote:
> 
> > > diff --git a/arch/x86/math-emu/reg_ld_str.c b/arch/x86/math-emu/reg_ld_str.c
> > > index f3779743d15e..fe6246ff9887 100644
> > > --- a/arch/x86/math-emu/reg_ld_str.c
> > > +++ b/arch/x86/math-emu/reg_ld_str.c
> > > @@ -85,7 +85,7 @@ int FPU_load_extended(long double __user *s, int stnr)
> > >
> > >       RE_ENTRANT_CHECK_OFF;
> > >       FPU_access_ok(s, 10);
> > > -     __copy_from_user(sti_ptr, s, 10);
> > > +     FPU_copy_from_user(sti_ptr, s, 10);
> >
> > These access_ok() checks seem redundant everywhere in this file (after
> > your switch from __copy* to copy*. I mean, I guess, just leave them, but
> > *shrug*
> 
> There have always been duplicate/inconsistent for the get_user/put_user
> case. I considered cleaning it all up but then decided to touch it as little
> as possible.

Yeah, at this point, I'd agree. :)

-- 
Kees Cook

[tip: x86/cpu] x86/math-emu: Check __copy_from_user() result

[tip-bot2 for Arnd Bergmann]

The following commit has been merged into the x86/cpu branch of tip:

Commit-ID:     e6b44ce1925a8329a937c57f0d60ba0d9bb5d226
Gitweb:        https://git.kernel.org/tip/e6b44ce1925a8329a937c57f0d60ba0d9bb5d226
Author:        Arnd Bergmann <arnd@arndb.de>
AuthorDate:    Tue, 01 Oct 2019 16:23:34 +02:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Thu, 03 Oct 2019 10:51:08 +02:00

x86/math-emu: Check __copy_from_user() result

The new __must_check annotation on __copy_from_user() successfully
identified some code that has lacked the check since at least
linux-2.1.73:

  arch/x86/math-emu/reg_ld_str.c:88:2: error: ignoring return value of \
  function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
          __copy_from_user(sti_ptr, s, 10);
          ^~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~
  arch/x86/math-emu/reg_ld_str.c:1129:2: error: ignoring return value of \
  function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
          __copy_from_user(register_base + offset, s, other);
          ^~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  arch/x86/math-emu/reg_ld_str.c:1131:3: error: ignoring return value of \
  function declared with 'warn_unused_result' attribute [-Werror,-Wunused-result]
                  __copy_from_user(register_base, s + other, offset);
                ^~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In addition, the get_user()/put_user() helpers do not enforce a return
value check, but actually still require one. These have been missing for
even longer.

Change the internal wrappers around get_user()/put_user() to force
a signal and add a corresponding wrapper around __copy_from_user()
to check all such cases.

 [ bp: Break long lines. ]

Fixes: 257e458057e5 ("Import 2.1.73")
Fixes: 9dd819a15162 ("uaccess: add missing __must_check attributes")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Kees Cook <keescook@chromium.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Bill Metzenthen <billm@melbpc.org.au>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: x86-ml <x86@kernel.org>
Link: https://lkml.kernel.org/r/20191001142344.1274185-1-arnd@arndb.de
---
 arch/x86/math-emu/fpu_system.h | 6 ++++--
 arch/x86/math-emu/reg_ld_str.c | 6 +++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/x86/math-emu/fpu_system.h b/arch/x86/math-emu/fpu_system.h
index f98a0c9..9b41391 100644
--- a/arch/x86/math-emu/fpu_system.h
+++ b/arch/x86/math-emu/fpu_system.h
@@ -107,6 +107,8 @@ static inline bool seg_writable(struct desc_struct *d)
 #define FPU_access_ok(y,z)	if ( !access_ok(y,z) ) \
 				math_abort(FPU_info,SIGSEGV)
 #define FPU_abort		math_abort(FPU_info, SIGSEGV)
+#define FPU_copy_from_user(to, from, n)	\
+		do { if (copy_from_user(to, from, n)) FPU_abort; } while (0)
 
 #undef FPU_IGNORE_CODE_SEGV
 #ifdef FPU_IGNORE_CODE_SEGV
@@ -122,7 +124,7 @@ static inline bool seg_writable(struct desc_struct *d)
 #define	FPU_code_access_ok(z) FPU_access_ok((void __user *)FPU_EIP,z)
 #endif
 
-#define FPU_get_user(x,y)       get_user((x),(y))
-#define FPU_put_user(x,y)       put_user((x),(y))
+#define FPU_get_user(x,y) do { if (get_user((x),(y))) FPU_abort; } while (0)
+#define FPU_put_user(x,y) do { if (put_user((x),(y))) FPU_abort; } while (0)
 
 #endif
diff --git a/arch/x86/math-emu/reg_ld_str.c b/arch/x86/math-emu/reg_ld_str.c
index f377974..fe6246f 100644
--- a/arch/x86/math-emu/reg_ld_str.c
+++ b/arch/x86/math-emu/reg_ld_str.c
@@ -85,7 +85,7 @@ int FPU_load_extended(long double __user *s, int stnr)
 
 	RE_ENTRANT_CHECK_OFF;
 	FPU_access_ok(s, 10);
-	__copy_from_user(sti_ptr, s, 10);
+	FPU_copy_from_user(sti_ptr, s, 10);
 	RE_ENTRANT_CHECK_ON;
 
 	return FPU_tagof(sti_ptr);
@@ -1126,9 +1126,9 @@ void frstor(fpu_addr_modes addr_modes, u_char __user *data_address)
 	/* Copy all registers in stack order. */
 	RE_ENTRANT_CHECK_OFF;
 	FPU_access_ok(s, 80);
-	__copy_from_user(register_base + offset, s, other);
+	FPU_copy_from_user(register_base + offset, s, other);
 	if (offset)
-		__copy_from_user(register_base, s + other, offset);
+		FPU_copy_from_user(register_base, s + other, offset);
 	RE_ENTRANT_CHECK_ON;
 
 	for (i = 0; i < 8; i++) {

[tip: x86/cpu] x86/math-emu: Limit MATH_EMULATION to 486SX compatibles

[tip-bot2 for Arnd Bergmann]

The following commit has been merged into the x86/cpu branch of tip:

Commit-ID:     87d6021b814353d7b353afcc3698ffe49de7d4ec
Gitweb:        https://git.kernel.org/tip/87d6021b814353d7b353afcc3698ffe49de7d4ec
Author:        Arnd Bergmann <arnd@arndb.de>
AuthorDate:    Tue, 01 Oct 2019 16:23:35 +02:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Thu, 03 Oct 2019 10:51:17 +02:00

x86/math-emu: Limit MATH_EMULATION to 486SX compatibles

The FPU emulation code is old and fragile in places, try to limit its
use to builds for CPUs that actually use it. As far as I can tell,
this is only true for i486sx compatibles, including the Cyrix 486SLC,
AMD Am486SX and ÉLAN SC410, UMC U5S amd DM&P VortexSX86, all of which
were relatively short-lived and got replaced with i486DX compatible
processors soon after introduction, though some of the embedded versions
remained available much longer.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Kees Cook <keescook@chromium.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Bill Metzenthen <billm@melbpc.org.au>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: x86-ml <x86@kernel.org>
Link: https://lkml.kernel.org/r/20191001142344.1274185-2-arnd@arndb.de
---
 arch/x86/Kconfig              |  2 +-
 arch/x86/Kconfig.cpu          | 25 ++++++++++++++++---------
 arch/x86/Makefile_32.cpu      |  1 +
 arch/x86/include/asm/module.h |  2 ++
 4 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d6e1faa..91c22ee 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1751,7 +1751,7 @@ config X86_RESERVE_LOW
 config MATH_EMULATION
 	bool
 	depends on MODIFY_LDT_SYSCALL
-	prompt "Math emulation" if X86_32
+	prompt "Math emulation" if X86_32 && (M486SX || MELAN)
 	---help---
 	  Linux can emulate a math coprocessor (used for floating point
 	  operations) if you don't have one. 486DX and Pentium processors have
diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu
index 8e29c99..af9c967 100644
--- a/arch/x86/Kconfig.cpu
+++ b/arch/x86/Kconfig.cpu
@@ -50,12 +50,19 @@ choice
 	  See each option's help text for additional details. If you don't know
 	  what to do, choose "486".
 
+config M486SX
+	bool "486SX"
+	depends on X86_32
+	---help---
+	  Select this for an 486-class CPU without an FPU such as
+	  AMD/Cyrix/IBM/Intel SL/SLC/SLC2/SLC3/SX/SX2 and UMC U5S.
+
 config M486
-	bool "486"
+	bool "486DX"
 	depends on X86_32
 	---help---
 	  Select this for an 486-class CPU such as AMD/Cyrix/IBM/Intel
-	  486DX/DX2/DX4 or SL/SLC/SLC2/SLC3/SX/SX2 and UMC U5D or U5S.
+	  486DX/DX2/DX4 and UMC U5D.
 
 config M586
 	bool "586/K5/5x86/6x86/6x86MX"
@@ -312,20 +319,20 @@ config X86_L1_CACHE_SHIFT
 	int
 	default "7" if MPENTIUM4 || MPSC
 	default "6" if MK7 || MK8 || MPENTIUMM || MCORE2 || MATOM || MVIAC7 || X86_GENERIC || GENERIC_CPU
-	default "4" if MELAN || M486 || MGEODEGX1
+	default "4" if MELAN || M486SX || M486 || MGEODEGX1
 	default "5" if MWINCHIP3D || MWINCHIPC6 || MCRUSOE || MEFFICEON || MCYRIXIII || MK6 || MPENTIUMIII || MPENTIUMII || M686 || M586MMX || M586TSC || M586 || MVIAC3_2 || MGEODE_LX
 
 config X86_F00F_BUG
 	def_bool y
-	depends on M586MMX || M586TSC || M586 || M486
+	depends on M586MMX || M586TSC || M586 || M486SX || M486
 
 config X86_INVD_BUG
 	def_bool y
-	depends on M486
+	depends on M486SX || M486
 
 config X86_ALIGNMENT_16
 	def_bool y
-	depends on MWINCHIP3D || MWINCHIPC6 || MCYRIXIII || MELAN || MK6 || M586MMX || M586TSC || M586 || M486 || MVIAC3_2 || MGEODEGX1
+	depends on MWINCHIP3D || MWINCHIPC6 || MCYRIXIII || MELAN || MK6 || M586MMX || M586TSC || M586 || M486SX || M486 || MVIAC3_2 || MGEODEGX1
 
 config X86_INTEL_USERCOPY
 	def_bool y
@@ -378,7 +385,7 @@ config X86_MINIMUM_CPU_FAMILY
 
 config X86_DEBUGCTLMSR
 	def_bool y
-	depends on !(MK6 || MWINCHIPC6 || MWINCHIP3D || MCYRIXIII || M586MMX || M586TSC || M586 || M486) && !UML
+	depends on !(MK6 || MWINCHIPC6 || MWINCHIP3D || MCYRIXIII || M586MMX || M586TSC || M586 || M486SX || M486) && !UML
 
 menuconfig PROCESSOR_SELECT
 	bool "Supported processor vendors" if EXPERT
@@ -402,7 +409,7 @@ config CPU_SUP_INTEL
 config CPU_SUP_CYRIX_32
 	default y
 	bool "Support Cyrix processors" if PROCESSOR_SELECT
-	depends on M486 || M586 || M586TSC || M586MMX || (EXPERT && !64BIT)
+	depends on M486SX || M486 || M586 || M586TSC || M586MMX || (EXPERT && !64BIT)
 	---help---
 	  This enables detection, tunings and quirks for Cyrix processors
 
@@ -470,7 +477,7 @@ config CPU_SUP_TRANSMETA_32
 config CPU_SUP_UMC_32
 	default y
 	bool "Support UMC processors" if PROCESSOR_SELECT
-	depends on M486 || (EXPERT && !64BIT)
+	depends on M486SX || M486 || (EXPERT && !64BIT)
 	---help---
 	  This enables detection, tunings and quirks for UMC processors
 
diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index 1f5faf8..cd30567 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -10,6 +10,7 @@ else
 tune		= $(call cc-option,-mcpu=$(1),$(2))
 endif
 
+cflags-$(CONFIG_M486SX)		+= -march=i486
 cflags-$(CONFIG_M486)		+= -march=i486
 cflags-$(CONFIG_M586)		+= -march=i586
 cflags-$(CONFIG_M586TSC)	+= -march=i586
diff --git a/arch/x86/include/asm/module.h b/arch/x86/include/asm/module.h
index 7948a17..c215d27 100644
--- a/arch/x86/include/asm/module.h
+++ b/arch/x86/include/asm/module.h
@@ -15,6 +15,8 @@ struct mod_arch_specific {
 
 #ifdef CONFIG_X86_64
 /* X86_64 does not define MODULE_PROC_FAMILY */
+#elif defined CONFIG_M486SX
+#define MODULE_PROC_FAMILY "486SX "
 #elif defined CONFIG_M486
 #define MODULE_PROC_FAMILY "486 "
 #elif defined CONFIG_M586