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.
Add a helper that verifies the feature string in filter_cpuid_features()
is non-zero, and also cleans up the open coded paths mentioned above.
Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
---
arch/x86/kernel/cpu/common.c | 25 ++++++++++++++++++++-----
arch/x86/kernel/cpu/cpuid-deps.c | 21 +++------------------
include/linux/cpu.h | 2 ++
3 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8d12c5722245..7aede0760ebc 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -697,7 +697,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), df->level);
}
}
@@ -1651,10 +1651,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));
taint++;
}
@@ -1972,6 +1969,24 @@ static void generic_identify(struct cpuinfo_x86 *c)
#endif
}
+const char *x86_cap_name(unsigned int bit)
+{
+ unsigned int word = bit >> 5;
+ static char undef_buf[16];
+ 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(undef_buf, sizeof(undef_buf), "%u:%u", word, bit & 31);
+ return undef_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..1106a5476dca 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 <linux/cpu.h>
#include <asm/cpufeature.h>
struct cpuid_dep {
@@ -156,24 +157,8 @@ 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];
const struct cpuid_dep *d;
for (d = cpuid_deps; d->feature; d++) {
@@ -185,8 +170,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),
+ x86_cap_name(d->depends));
}
}
}
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 487b3bf2e1ea..8b2176561f29 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -229,4 +229,6 @@ static inline bool cpu_attack_vector_mitigated(enum cpu_attack_vectors v)
#define smt_mitigations SMT_MITIGATIONS_OFF
#endif
+const char *x86_cap_name(unsigned int bit);
+
#endif /* _LINUX_CPU_H_ */
--
2.53.0
On 2/12/2026 7:35 AM, Maciej Wieczor-Retman wrote:
>
> +const char *x86_cap_name(unsigned int bit)
> +{
> + unsigned int word = bit >> 5;
> + static char undef_buf[16];
> + 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(undef_buf, sizeof(undef_buf), "%u:%u", word, bit & 31);
> + return undef_buf;
> +}
> +
Isn't it unsafe to return undef_buf because the pointer is local to this
function even though it is marked static?
For example, the consecutive calls below would overwrite the value
stored at that address.
> 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),
> + x86_cap_name(d->depends));
> }
> }
> }
On 2026-02-12 at 16:28:10 -0800, Sohil Mehta wrote:
>On 2/12/2026 7:35 AM, Maciej Wieczor-Retman wrote:
>
>>
>> +const char *x86_cap_name(unsigned int bit)
>> +{
>> + unsigned int word = bit >> 5;
>> + static char undef_buf[16];
>> + 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(undef_buf, sizeof(undef_buf), "%u:%u", word, bit & 31);
>> + return undef_buf;
>> +}
>> +
>
>Isn't it unsafe to return undef_buf because the pointer is local to this
>function even though it is marked static?
>
>For example, the consecutive calls below would overwrite the value
>stored at that address.
I just rechecked and after I caused that pr_warn_once() to be executed the
strings are printed correctly.
Looking a bit further down, the pointers of the returned const char* are two
distinct values. So I assume because there are two calls to x86_cap_name() in
the pr_warn_once() the two static undef_buf char arrays are statically allocated
and there is no overwriting, but please correct me if that assumption is wrong.
>> 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),
>> + x86_cap_name(d->depends));
>> }
>> }
>> }
--
Kind regards
Maciej Wieczór-Retman
On 2026-02-13 02:02, Maciej Wieczor-Retman wrote:
> On 2026-02-12 at 16:28:10 -0800, Sohil Mehta wrote:
>> On 2/12/2026 7:35 AM, Maciej Wieczor-Retman wrote:
>>
>>>
>>> +const char *x86_cap_name(unsigned int bit)
>>> +{
>>> + unsigned int word = bit >> 5;
>>> + static char undef_buf[16];
>>> + 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(undef_buf, sizeof(undef_buf), "%u:%u", word, bit & 31);
>>> + return undef_buf;
>>> +}
>>> +
>>
>> Isn't it unsafe to return undef_buf because the pointer is local to this
>> function even though it is marked static?
>>
>> For example, the consecutive calls below would overwrite the value
>> stored at that address.
>
> I just rechecked and after I caused that pr_warn_once() to be executed the
> strings are printed correctly.
>
> Looking a bit further down, the pointers of the returned const char* are two
> distinct values. So I assume because there are two calls to x86_cap_name() in
> the pr_warn_once() the two static undef_buf char arrays are statically allocated
> and there is no overwriting, but please correct me if that assumption is wrong.
>
At the very least the buffer would need to be percpu for it to be safe. On the
other hand, there aren't a whole lot of places that uses this and the buffer
needed isn't that big, so the best way is probably to have the caller pass in
a buffer pointer on the stack. The callers are typically going to be large
functions which require a stack frame anyway.
-hpa
On 2026-02-13 at 12:15:24 -0800, H. Peter Anvin wrote:
>On 2026-02-13 02:02, Maciej Wieczor-Retman wrote:
>> On 2026-02-12 at 16:28:10 -0800, Sohil Mehta wrote:
>>> On 2/12/2026 7:35 AM, Maciej Wieczor-Retman wrote:
>>>
>>>>
>>>> +const char *x86_cap_name(unsigned int bit)
>>>> +{
>>>> + unsigned int word = bit >> 5;
>>>> + static char undef_buf[16];
>>>> + 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(undef_buf, sizeof(undef_buf), "%u:%u", word, bit & 31);
>>>> + return undef_buf;
>>>> +}
>>>> +
>>>
>>> Isn't it unsafe to return undef_buf because the pointer is local to this
>>> function even though it is marked static?
>>>
>>> For example, the consecutive calls below would overwrite the value
>>> stored at that address.
>>
>> I just rechecked and after I caused that pr_warn_once() to be executed the
>> strings are printed correctly.
>>
>> Looking a bit further down, the pointers of the returned const char* are two
>> distinct values. So I assume because there are two calls to x86_cap_name() in
>> the pr_warn_once() the two static undef_buf char arrays are statically allocated
>> and there is no overwriting, but please correct me if that assumption is wrong.
>>
>
>At the very least the buffer would need to be percpu for it to be safe. On the
>other hand, there aren't a whole lot of places that uses this and the buffer
>needed isn't that big, so the best way is probably to have the caller pass in
>a buffer pointer on the stack. The callers are typically going to be large
>functions which require a stack frame anyway.
>
> -hpa
>
Sure, I'll change it to pass a pointer.
--
Kind regards
Maciej Wieczór-Retman
On 2/13/2026 2:02 AM, Maciej Wieczor-Retman wrote:
> On 2026-02-12 at 16:28:10 -0800, Sohil Mehta wrote:
>> On 2/12/2026 7:35 AM, Maciej Wieczor-Retman wrote:
>>
>>>
>>> +const char *x86_cap_name(unsigned int bit)
>>> +{
>>> + unsigned int word = bit >> 5;
>>> + static char undef_buf[16];
>>> + 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(undef_buf, sizeof(undef_buf), "%u:%u", word, bit & 31);
>>> + return undef_buf;
>>> +}
>>> +
>>
>> Isn't it unsafe to return undef_buf because the pointer is local to this
>> function even though it is marked static?
>>
>> For example, the consecutive calls below would overwrite the value
>> stored at that address.
>
> I just rechecked and after I caused that pr_warn_once() to be executed the
> strings are printed correctly.
>
The issue would only happen if both d->feature and d->depends don't have
anything in x86_cap_flags[]. Was that true for your test?
> Looking a bit further down, the pointers of the returned const char* are two
> distinct values. So I assume because there are two calls to x86_cap_name() in
> the pr_warn_once() the two static undef_buf char arrays are statically allocated
> and there is no overwriting, but please correct me if that assumption is wrong.
>
I don't think it works that way. As the variable is defined as "static",
only a single undef_buf is allocated. So the same pointer should be
returned after every call. I added the below dependency and it prints
the same feature twice.
"x86 CPU feature dependency check failure: CPU0 has '12:19' enabled but
'12:19' disabled. Kernel..."
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c
b/arch/x86/kernel/cpu/cpuid-deps.c
index 1106a5476dca..51c482c18a2c 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -93,6 +93,7 @@ static const struct cpuid_dep cpuid_deps[] = {
{ X86_FEATURE_FRED, X86_FEATURE_LKGS },
{ X86_FEATURE_SPEC_CTRL_SSBD, X86_FEATURE_SPEC_CTRL },
{ X86_FEATURE_LASS, X86_FEATURE_SMAP },
+ { X86_FEATURE_WRMSRNS, X86_FEATURE_FZRM},
{}
};
>>> 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),
>>> + x86_cap_name(d->depends));
>>> }
>>> }
>>> }
>
On 2026-02-13 at 10:22:31 -0800, Sohil Mehta wrote:
>On 2/13/2026 2:02 AM, Maciej Wieczor-Retman wrote:
>> On 2026-02-12 at 16:28:10 -0800, Sohil Mehta wrote:
>>> On 2/12/2026 7:35 AM, Maciej Wieczor-Retman wrote:
>>>
>>>>
>>>> +const char *x86_cap_name(unsigned int bit)
>>>> +{
>>>> + unsigned int word = bit >> 5;
>>>> + static char undef_buf[16];
>>>> + 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(undef_buf, sizeof(undef_buf), "%u:%u", word, bit & 31);
>>>> + return undef_buf;
>>>> +}
>>>> +
>>>
>>> Isn't it unsafe to return undef_buf because the pointer is local to this
>>> function even though it is marked static?
>>>
>>> For example, the consecutive calls below would overwrite the value
>>> stored at that address.
>>
>> I just rechecked and after I caused that pr_warn_once() to be executed the
>> strings are printed correctly.
>>
>
>The issue would only happen if both d->feature and d->depends don't have
>anything in x86_cap_flags[]. Was that true for your test?
Ah, my bad, you're right, my test just returned the proper name not the word:bit
part.
>
>> Looking a bit further down, the pointers of the returned const char* are two
>> distinct values. So I assume because there are two calls to x86_cap_name() in
>> the pr_warn_once() the two static undef_buf char arrays are statically allocated
>> and there is no overwriting, but please correct me if that assumption is wrong.
>>
>
>I don't think it works that way. As the variable is defined as "static",
>only a single undef_buf is allocated. So the same pointer should be
>returned after every call. I added the below dependency and it prints
>the same feature twice.
>
>"x86 CPU feature dependency check failure: CPU0 has '12:19' enabled but
>'12:19' disabled. Kernel..."
>
>diff --git a/arch/x86/kernel/cpu/cpuid-deps.c
>b/arch/x86/kernel/cpu/cpuid-deps.c
>index 1106a5476dca..51c482c18a2c 100644
>--- a/arch/x86/kernel/cpu/cpuid-deps.c
>+++ b/arch/x86/kernel/cpu/cpuid-deps.c
>@@ -93,6 +93,7 @@ static const struct cpuid_dep cpuid_deps[] = {
> { X86_FEATURE_FRED, X86_FEATURE_LKGS },
> { X86_FEATURE_SPEC_CTRL_SSBD, X86_FEATURE_SPEC_CTRL },
> { X86_FEATURE_LASS, X86_FEATURE_SMAP },
>+ { X86_FEATURE_WRMSRNS, X86_FEATURE_FZRM},
> {}
> };
So I'd think pr_cont() would work here best? Just tested it on your example and
it does work. Of course that quirk would have to be documented above the
function definition. Not sure if there is a better way while still keeping the
convenience of not having to pass any char pointers to the x86_cap_name():
pr_warn_once("x86 CPU feature dependency check failure: CPU%d has '%s' enabled ", smp_processor_id(),
x86_cap_name(d->feature));
pr_cont("but '%s' disabled. Kernel might be fine, but no guarantees.\n", x86_cap_name(d->depends));
x86 CPU feature dependency check failure: CPU0 has '12:19'
enabled but '12:10' disabled. Kernel might be fine, but no guarantees.
>
>>>> 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),
>>>> + x86_cap_name(d->depends));
>>>> }
>>>> }
>>>> }
>>
>
--
Kind regards
Maciej Wieczór-Retman
On 2/13/2026 11:10 AM, Maciej Wieczor-Retman wrote: > So I'd think pr_cont() would work here best? What about multi-threading? Would x86_cap_name() be called at the same time from multiple CPUs?
On 2026-02-13 at 12:00:16 -0800, Sohil Mehta wrote: >On 2/13/2026 11:10 AM, Maciej Wieczor-Retman wrote: >> So I'd think pr_cont() would work here best? > >What about multi-threading? Would x86_cap_name() be called at the same >time from multiple CPUs? I guess you're right, there are too many ways this can backfire. I'll do what Peter suggested and just try with passing a pointer from the calling function. -- Kind regards Maciej Wieczór-Retman
© 2016 - 2026 Red Hat, Inc.