From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
In filter_cpuid_features, x86_cap_flags[] is read, but it's not verified
whether the string is non-zero which could lead to unwanted output.
In two more places there are open coded paths that try to retrieve a
feature string, and if there isn't one, the feature word and bit are
returned instead. While correcting filter_cpuid_features() with a helper
it's trivial to also clean up these open coded cases.
Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
Changelog v8:
- Move x86_cap_name() declaration from linux/cpu.h to the arch/cpu.h.
Include arch/cpu.h in the cpuid-deps.c file instead of linux/cpu.h.
Changelog v7:
- sizeof(buf) -> 16
- Rebase onto 7.01-rc1.
Changelog v6:
- Remove parts of the patch message that are redundant and just copy
what's visible in the diff.
- Redo the helper to use an external char buffer instead of a local
static string.
arch/x86/include/asm/cpu.h | 2 ++
arch/x86/kernel/cpu/common.c | 26 +++++++++++++++++++++-----
arch/x86/kernel/cpu/cpuid-deps.c | 20 +++-----------------
3 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index ad235dda1ded..fdf9566e2272 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -67,4 +67,6 @@ int intel_microcode_sanity_check(void *mc, bool print_err, int hdr_type);
extern struct cpumask cpus_stop_mask;
+const char *x86_cap_name(unsigned int bit, char *buf);
+
#endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 9aa11224a038..b60269174d95 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -675,6 +675,7 @@ cpuid_dependent_features[] = {
static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
{
const struct cpuid_dependent_feature *df;
+ char feature_buf[16];
for (df = cpuid_dependent_features; df->feature; df++) {
@@ -697,7 +698,7 @@ static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
continue;
pr_warn("CPU: CPU feature %s disabled, no CPUID level 0x%x\n",
- x86_cap_flags[df->feature], df->level);
+ x86_cap_name(df->feature, feature_buf), df->level);
}
}
@@ -1634,6 +1635,7 @@ static inline bool parse_set_clear_cpuid(char *arg, bool set)
while (arg) {
bool found __maybe_unused = false;
+ char name_buf[16];
unsigned int bit;
opt = strsep(&arg, ",");
@@ -1654,10 +1656,7 @@ static inline bool parse_set_clear_cpuid(char *arg, bool set)
setup_clear_cpu_cap(bit);
}
/* empty-string, i.e., ""-defined feature flags */
- if (!x86_cap_flags[bit])
- pr_cont(" %d:%d\n", bit >> 5, bit & 31);
- else
- pr_cont(" %s\n", x86_cap_flags[bit]);
+ pr_cont(" %s\n", x86_cap_name(bit, name_buf));
taint++;
}
@@ -1980,6 +1979,23 @@ static void generic_identify(struct cpuinfo_x86 *c)
#endif
}
+const char *x86_cap_name(unsigned int bit, char *buf)
+{
+ unsigned int word = bit >> 5;
+ const char *name = NULL;
+
+ if (likely(word < NCAPINTS))
+ name = x86_cap_flags[bit];
+ else if (likely(word < NCAPINTS + NBUGINTS))
+ name = x86_bug_flags[bit - 32 * NCAPINTS];
+
+ if (name)
+ return name;
+
+ snprintf(buf, 16, "%u:%u", word, bit & 31);
+ return buf;
+}
+
/*
* This does the hard work of actually picking apart the CPU stuff...
*/
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 146f6f8b0650..dfd79b06ab7b 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -2,6 +2,7 @@
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/module.h>
+#include <asm/cpu.h>
#include <asm/cpufeature.h>
struct cpuid_dep {
@@ -156,21 +157,6 @@ void setup_clear_cpu_cap(unsigned int feature)
do_clear_cpu_cap(NULL, feature);
}
-/*
- * Return the feature "name" if available, otherwise return
- * the X86_FEATURE_* numerals to make it easier to identify
- * the feature.
- */
-static const char *x86_feature_name(unsigned int feature, char *buf)
-{
- if (x86_cap_flags[feature])
- return x86_cap_flags[feature];
-
- snprintf(buf, 16, "%d*32+%2d", feature / 32, feature % 32);
-
- return buf;
-}
-
void check_cpufeature_deps(struct cpuinfo_x86 *c)
{
char feature_buf[16], depends_buf[16];
@@ -185,8 +171,8 @@ void check_cpufeature_deps(struct cpuinfo_x86 *c)
*/
pr_warn_once("x86 CPU feature dependency check failure: CPU%d has '%s' enabled but '%s' disabled. Kernel might be fine, but no guarantees.\n",
smp_processor_id(),
- x86_feature_name(d->feature, feature_buf),
- x86_feature_name(d->depends, depends_buf));
+ x86_cap_name(d->feature, feature_buf),
+ x86_cap_name(d->depends, depends_buf));
}
}
}
--
2.53.0On 3/2/2026 7:25 AM, Maciej Wieczor-Retman wrote:
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 9aa11224a038..b60269174d95 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -675,6 +675,7 @@ cpuid_dependent_features[] = {
> static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
> {
> const struct cpuid_dependent_feature *df;
> + char feature_buf[16];
>
The usage of number 16 isn't intuitive here. Might be useful to use a
macro here. See below..
> for (df = cpuid_dependent_features; df->feature; df++) {
>
> @@ -697,7 +698,7 @@ static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
> continue;
>
> pr_warn("CPU: CPU feature %s disabled, no CPUID level 0x%x\n",
> - x86_cap_flags[df->feature], df->level);
> + x86_cap_name(df->feature, feature_buf), df->level);
> }
> }
>
> @@ -1634,6 +1635,7 @@ static inline bool parse_set_clear_cpuid(char *arg, bool set)
>
> while (arg) {
> bool found __maybe_unused = false;
> + char name_buf[16];
> unsigned int bit;
>
> opt = strsep(&arg, ",");
> @@ -1654,10 +1656,7 @@ static inline bool parse_set_clear_cpuid(char *arg, bool set)
> setup_clear_cpu_cap(bit);
> }
> /* empty-string, i.e., ""-defined feature flags */
> - if (!x86_cap_flags[bit])
> - pr_cont(" %d:%d\n", bit >> 5, bit & 31);
> - else
> - pr_cont(" %s\n", x86_cap_flags[bit]);
> + pr_cont(" %s\n", x86_cap_name(bit, name_buf));
>
> taint++;
> }
> @@ -1980,6 +1979,23 @@ static void generic_identify(struct cpuinfo_x86 *c)
> #endif
> }
>
It would be useful to have a comment here because the function is called
from multiple files. You can probably reuse the one from the deleted
x86_feature_name().
/*
* Return the feature "name" if available, otherwise return
* the X86_FEATURE_* numerals to make it easier to identify
* the feature.
*/
> +const char *x86_cap_name(unsigned int bit, char *buf)
> +{
> + unsigned int word = bit >> 5;
> + const char *name = NULL;
> +
> + if (likely(word < NCAPINTS))
> + name = x86_cap_flags[bit];
> + else if (likely(word < NCAPINTS + NBUGINTS))
> + name = x86_bug_flags[bit - 32 * NCAPINTS];
> +
> + if (name)
> + return name;
> +
> + snprintf(buf, 16, "%u:%u", word, bit & 31);
The buffer size of 16 is expected to be in sync with the callers and
easy to mess up. Also, the callers wouldn't know that it needs to be 16
because of this snprintf(). Should we have a simple define for it?
Maybe something like,
#define X86_CAP_BUF_SIZE 16
> + return buf;
> +}
> +
> /*
> * This does the hard work of actually picking apart the CPU stuff...
> */
> diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
> index 146f6f8b0650..dfd79b06ab7b 100644
> --- a/arch/x86/kernel/cpu/cpuid-deps.c
> +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> @@ -2,6 +2,7 @@
> #include <linux/kernel.h>
> #include <linux/init.h>
> #include <linux/module.h>
Nit:
You can consider adding a newline here.
> +#include <asm/cpu.h>
> #include <asm/cpufeature.h>
>
On 2026-03-09 at 23:23:26 -0700, Sohil Mehta wrote:
>On 3/2/2026 7:25 AM, Maciej Wieczor-Retman wrote:
>
>> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
>> index 9aa11224a038..b60269174d95 100644
>> --- a/arch/x86/kernel/cpu/common.c
>> +++ b/arch/x86/kernel/cpu/common.c
>> @@ -675,6 +675,7 @@ cpuid_dependent_features[] = {
>> static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
>> {
>> const struct cpuid_dependent_feature *df;
>> + char feature_buf[16];
>>
>
>The usage of number 16 isn't intuitive here. Might be useful to use a
>macro here. See below..
>
...
>
>It would be useful to have a comment here because the function is called
>from multiple files. You can probably reuse the one from the deleted
>x86_feature_name().
Sure, that's a good idea.
>
>/*
> * Return the feature "name" if available, otherwise return
> * the X86_FEATURE_* numerals to make it easier to identify
> * the feature.
> */
>
>> +const char *x86_cap_name(unsigned int bit, char *buf)
>> +{
>> + unsigned int word = bit >> 5;
>> + const char *name = NULL;
>> +
>> + if (likely(word < NCAPINTS))
>> + name = x86_cap_flags[bit];
>> + else if (likely(word < NCAPINTS + NBUGINTS))
>> + name = x86_bug_flags[bit - 32 * NCAPINTS];
>> +
>> + if (name)
>> + return name;
>> +
>> + snprintf(buf, 16, "%u:%u", word, bit & 31);
>
>The buffer size of 16 is expected to be in sync with the callers and
>easy to mess up. Also, the callers wouldn't know that it needs to be 16
>because of this snprintf(). Should we have a simple define for it?
>
>Maybe something like,
>
>#define X86_CAP_BUF_SIZE 16
Yes, thanks, I probably should've done that frome the start :)
>
>> + return buf;
>> +}
>> +
>> /*
>> * This does the hard work of actually picking apart the CPU stuff...
>> */
>> diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
>> index 146f6f8b0650..dfd79b06ab7b 100644
>> --- a/arch/x86/kernel/cpu/cpuid-deps.c
>> +++ b/arch/x86/kernel/cpu/cpuid-deps.c
>> @@ -2,6 +2,7 @@
>> #include <linux/kernel.h>
>> #include <linux/init.h>
>> #include <linux/module.h>
>
>Nit:
>
>You can consider adding a newline here.
Okay.
>> +#include <asm/cpu.h>
>> #include <asm/cpufeature.h>
>>
--
Kind regards
Maciej Wieczór-Retman
© 2016 - 2026 Red Hat, Inc.