* [PATCH 2/2] x86: math-emu: limit MATH_EMULATION to 486SX compatibles
2019-10-01 14:23 [PATCH 1/2] x86: math-emu: check __copy_from_user result Arnd Bergmann
@ 2019-10-01 14:23 ` Arnd Bergmann
2019-10-01 21:54 ` Kees Cook
2019-10-03 9:33 ` [tip: x86/cpu] x86/math-emu: Limit " tip-bot2 for Arnd Bergmann
2019-10-01 23:39 ` [PATCH 1/2] x86: math-emu: check __copy_from_user result Kees Cook
2019-10-03 9:33 ` [tip: x86/cpu] x86/math-emu: Check __copy_from_user() result tip-bot2 for Arnd Bergmann
2 siblings, 2 replies; 8+ messages in thread
From: Arnd Bergmann @ 2019-10-01 14:23 UTC (permalink / raw)
To: Bill Metzenthen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86
Cc: Arnd Bergmann, H. Peter Anvin, Andrew Morton, Masahiro Yamada,
linux-kernel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] x86: math-emu: limit MATH_EMULATION to 486SX compatibles
2019-10-01 14:23 ` [PATCH 2/2] x86: math-emu: limit MATH_EMULATION to 486SX compatibles Arnd Bergmann
@ 2019-10-01 21:54 ` Kees Cook
2019-10-03 9:33 ` [tip: x86/cpu] x86/math-emu: Limit " tip-bot2 for Arnd Bergmann
1 sibling, 0 replies; 8+ messages in thread
From: Kees Cook @ 2019-10-01 21:54 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Bill Metzenthen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, H. Peter Anvin, Andrew Morton, Masahiro Yamada,
linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip: x86/cpu] x86/math-emu: Limit MATH_EMULATION to 486SX compatibles
2019-10-01 14:23 ` [PATCH 2/2] x86: math-emu: limit MATH_EMULATION to 486SX compatibles Arnd Bergmann
2019-10-01 21:54 ` Kees Cook
@ 2019-10-03 9:33 ` tip-bot2 for Arnd Bergmann
1 sibling, 0 replies; 8+ messages in thread
From: tip-bot2 for Arnd Bergmann @ 2019-10-03 9:33 UTC (permalink / raw)
To: linux-tip-commits
Cc: Arnd Bergmann, Borislav Petkov, Kees Cook, H. Peter Anvin,
Andrew Morton, Bill Metzenthen, Ingo Molnar, Masahiro Yamada,
Thomas Gleixner, x86-ml, Ingo Molnar, Borislav Petkov,
linux-kernel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] x86: math-emu: check __copy_from_user result
2019-10-01 14:23 [PATCH 1/2] x86: math-emu: check __copy_from_user result Arnd Bergmann
2019-10-01 14:23 ` [PATCH 2/2] x86: math-emu: limit MATH_EMULATION to 486SX compatibles Arnd Bergmann
@ 2019-10-01 23:39 ` Kees Cook
2019-10-02 7:11 ` Arnd Bergmann
2019-10-03 9:33 ` [tip: x86/cpu] x86/math-emu: Check __copy_from_user() result tip-bot2 for Arnd Bergmann
2 siblings, 1 reply; 8+ messages in thread
From: Kees Cook @ 2019-10-01 23:39 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Bill Metzenthen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
x86, H. Peter Anvin, linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] x86: math-emu: check __copy_from_user result
2019-10-01 23:39 ` [PATCH 1/2] x86: math-emu: check __copy_from_user result Kees Cook
@ 2019-10-02 7:11 ` Arnd Bergmann
2019-10-03 6:26 ` Kees Cook
0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2019-10-02 7:11 UTC (permalink / raw)
To: Kees Cook
Cc: Bill Metzenthen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
the arch/x86 maintainers, H. Peter Anvin, linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] x86: math-emu: check __copy_from_user result
2019-10-02 7:11 ` Arnd Bergmann
@ 2019-10-03 6:26 ` Kees Cook
0 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2019-10-03 6:26 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Bill Metzenthen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
the arch/x86 maintainers, H. Peter Anvin, linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip: x86/cpu] x86/math-emu: Check __copy_from_user() result
2019-10-01 14:23 [PATCH 1/2] x86: math-emu: check __copy_from_user result Arnd Bergmann
2019-10-01 14:23 ` [PATCH 2/2] x86: math-emu: limit MATH_EMULATION to 486SX compatibles Arnd Bergmann
2019-10-01 23:39 ` [PATCH 1/2] x86: math-emu: check __copy_from_user result Kees Cook
@ 2019-10-03 9:33 ` tip-bot2 for Arnd Bergmann
2 siblings, 0 replies; 8+ messages in thread
From: tip-bot2 for Arnd Bergmann @ 2019-10-03 9:33 UTC (permalink / raw)
To: linux-tip-commits
Cc: Arnd Bergmann, Borislav Petkov, Kees Cook, H. Peter Anvin,
Bill Metzenthen, Ingo Molnar, Thomas Gleixner, x86-ml,
Ingo Molnar, Borislav Petkov, linux-kernel
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++) {
^ permalink raw reply related [flat|nested] 8+ messages in thread