From a24d61c609813963aacc9f6ec8343f4fcaac7243 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Thu, 2 Nov 2023 17:49:01 +0000 Subject: [PATCH 1/3] x86/lib: Fix overflow when counting digits tl;dr: The num_digits() function has a theoretical overflow issue. But it doesn't affect any actual in-tree users. Fix it by using a larger type for one of the local variables. Long version: There is an overflow in variable m in function num_digits when val is >= 1410065408 which leads to the digit calculation loop to iterate more times than required. This results in either more digits being counted or in some cases (for example where val is 1932683193) the value of m eventually overflows to zero and the while loop spins forever). Currently the function num_digits is currently only being used for small values of val in the SMP boot stage for digit counting on the number of cpus and NUMA nodes, so the overflow is never encountered. However it is useful to fix the overflow issue in case the function is used for other purposes in the future. (The issue was discovered while investigating the digit counting performance in various kernel helper functions rather than any real-world use-case). The simplest fix is to make m a long long, the overhead in multiplication speed for a long long is very minor for small values of val less than 10000 on modern processors. The alternative fix is to replace the multiplication with a constant division by 10 loop (this compiles down to an multiplication and shift) without needing to make m a long long, but this is slightly slower than the fix in this commit when measured on a range of x86 processors). [ dhansen: subject and changelog tweaks ] Fixes: 646e29a1789a ("x86: Improve the printout of the SMP bootup CPU table") Signed-off-by: Colin Ian King Signed-off-by: Dave Hansen Link: https://lore.kernel.org/all/20231102174901.2590325-1-colin.i.king%40gmail.com --- arch/x86/lib/misc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/lib/misc.c b/arch/x86/lib/misc.c index 92cd8ecc3a2c..40b81c338ae5 100644 --- a/arch/x86/lib/misc.c +++ b/arch/x86/lib/misc.c @@ -8,7 +8,7 @@ */ int num_digits(int val) { - int m = 10; + long long m = 10; int d = 1; if (val < 0) { From 79c603ee43b2674fba0257803bab265147821955 Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 8 Dec 2023 22:57:33 +0100 Subject: [PATCH 2/3] Documentation/x86: Document what /proc/cpuinfo is for This has been long overdue. Write down what x86's version of /proc/cpuinfo is and should be used for. With improvements by dhansen. Signed-off-by: Borislav Petkov (AMD) Reviewed-by: Dave Hansen Link: https://lore.kernel.org/r/20231129101700.28482-1-bp@alien8.de --- Documentation/arch/x86/cpuinfo.rst | 85 +++++++++++++++++++++++------- 1 file changed, 66 insertions(+), 19 deletions(-) diff --git a/Documentation/arch/x86/cpuinfo.rst b/Documentation/arch/x86/cpuinfo.rst index 08246e8ac835..8895784d4784 100644 --- a/Documentation/arch/x86/cpuinfo.rst +++ b/Documentation/arch/x86/cpuinfo.rst @@ -7,27 +7,74 @@ x86 Feature Flags Introduction ============ -On x86, flags appearing in /proc/cpuinfo have an X86_FEATURE definition -in arch/x86/include/asm/cpufeatures.h. If the kernel cares about a feature -or KVM want to expose the feature to a KVM guest, it can and should have -an X86_FEATURE_* defined. These flags represent hardware features as -well as software features. +The list of feature flags in /proc/cpuinfo is not complete and +represents an ill-fated attempt from long time ago to put feature flags +in an easy to find place for userspace. -If users want to know if a feature is available on a given system, they -try to find the flag in /proc/cpuinfo. If a given flag is present, it -means that the kernel supports it and is currently making it available. -If such flag represents a hardware feature, it also means that the -hardware supports it. +However, the amount of feature flags is growing by the CPU generation, +leading to unparseable and unwieldy /proc/cpuinfo. + +What is more, those feature flags do not even need to be in that file +because userspace doesn't care about them - glibc et al already use +CPUID to find out what the target machine supports and what not. + +And even if it doesn't show a particular feature flag - although the CPU +still does have support for the respective hardware functionality and +said CPU supports CPUID faulting - userspace can simply probe for the +feature and figure out if it is supported or not, regardless of whether +it is being advertised somewhere. + +Furthermore, those flag strings become an ABI the moment they appear +there and maintaining them forever when nothing even uses them is a lot +of wasted effort. + +So, the current use of /proc/cpuinfo is to show features which the +kernel has *enabled* and *supports*. As in: the CPUID feature flag is +there, there's an additional setup which the kernel has done while +booting and the functionality is ready to use. A perfect example for +that is "user_shstk" where additional code enablement is present in the +kernel to support shadow stack for user programs. + +So, if users want to know if a feature is available on a given system, +they try to find the flag in /proc/cpuinfo. If a given flag is present, +it means that + +* the kernel knows about the feature enough to have an X86_FEATURE bit + +* the kernel supports it and is currently making it available either to + userspace or some other part of the kernel + +* if the flag represents a hardware feature the hardware supports it. + +The absence of a flag in /proc/cpuinfo by itself means almost nothing to +an end user. + +On the one hand, a feature like "vaes" might be fully available to user +applications on a kernel that has not defined X86_FEATURE_VAES and thus +there is no "vaes" in /proc/cpuinfo. + +On the other hand, a new kernel running on non-VAES hardware would also +have no "vaes" in /proc/cpuinfo. There's no way for an application or +user to tell the difference. + +The end result is that the flags field in /proc/cpuinfo is marginally +useful for kernel debugging, but not really for anything else. +Applications should instead use things like the glibc facilities for +querying CPU support. Users should rely on tools like +tools/arch/x86/kcpuid and cpuid(1). + +Regarding implementation, flags appearing in /proc/cpuinfo have an +X86_FEATURE definition in arch/x86/include/asm/cpufeatures.h. These flags +represent hardware features as well as software features. + +If the kernel cares about a feature or KVM want to expose the feature to +a KVM guest, it should only then expose it to the guest when the guest +needs to parse /proc/cpuinfo. Which, as mentioned above, is highly +unlikely. KVM can synthesize the CPUID bit and the KVM guest can simply +query CPUID and figure out what the hypervisor supports and what not. As +already stated, /proc/cpuinfo is not a dumping ground for useless +feature flags. -If the expected flag does not appear in /proc/cpuinfo, things are murkier. -Users need to find out the reason why the flag is missing and find the way -how to enable it, which is not always easy. There are several factors that -can explain missing flags: the expected feature failed to enable, the feature -is missing in hardware, platform firmware did not enable it, the feature is -disabled at build or run time, an old kernel is in use, or the kernel does -not support the feature and thus has not enabled it. In general, /proc/cpuinfo -shows features which the kernel supports. For a full list of CPUID flags -which the CPU supports, use tools/arch/x86/kcpuid. How are feature flags created? ============================== From f789383fa34a266d0c1a76f272043a15a8edf733 Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Thu, 30 Nov 2023 16:39:33 +0100 Subject: [PATCH 3/3] x86/ia32: State that IA32 emulation is disabled Issue a short message once, on the first try to load a 32-bit process to save people time when wondering why it won't load and trying to execute it, would say: -bash: ./strsep32: cannot execute binary file: Exec format error Signed-off-by: Borislav Petkov (AMD) Reviewed-by: Nikolay Borisov Link: https://lore.kernel.org/r/20231130155213.1407-1-bp@alien8.de --- arch/x86/include/asm/elf.h | 2 +- arch/x86/include/asm/ia32.h | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h index a0234dfd1031..1e16bd5ac781 100644 --- a/arch/x86/include/asm/elf.h +++ b/arch/x86/include/asm/elf.h @@ -150,7 +150,7 @@ do { \ ((x)->e_machine == EM_X86_64) #define compat_elf_check_arch(x) \ - ((elf_check_arch_ia32(x) && ia32_enabled()) || \ + ((elf_check_arch_ia32(x) && ia32_enabled_verbose()) || \ (IS_ENABLED(CONFIG_X86_X32_ABI) && (x)->e_machine == EM_X86_64)) static inline void elf_common_init(struct thread_struct *t, diff --git a/arch/x86/include/asm/ia32.h b/arch/x86/include/asm/ia32.h index 5a2ae24b1204..094886a8717e 100644 --- a/arch/x86/include/asm/ia32.h +++ b/arch/x86/include/asm/ia32.h @@ -2,7 +2,6 @@ #ifndef _ASM_X86_IA32_H #define _ASM_X86_IA32_H - #ifdef CONFIG_IA32_EMULATION #include @@ -84,4 +83,14 @@ static inline bool ia32_enabled(void) #endif +static inline bool ia32_enabled_verbose(void) +{ + bool enabled = ia32_enabled(); + + if (IS_ENABLED(CONFIG_IA32_EMULATION) && !enabled) + pr_notice_once("32-bit emulation disabled. You can reenable with ia32_emulation=on\n"); + + return enabled; +} + #endif /* _ASM_X86_IA32_H */