Re: [tip:x86/cpu] x86/cpu: Strip any /proc/ cpuinfo model name field whitespace

H. Peter Anvin
2015-05-28T16:59:50Z

Why?!

We are taking about 48 bytes run once per cpu. It isn't worth it to optimize, in fact the extra code size hurts more.

On May 28, 2015 5:58:19 AM PDT, Borislav Petkov <bp@alien8.de> wrote:
>On Thu, May 28, 2015 at 01:32:29PM +0200, Borislav Petkov wrote:
>> + while (*p) {
>> + /* Note the last non-whitespace index */
>> + if (!isspace(*p))
>> + s = q;
>> +
>> + *q++ = *p++;
>
>This should be optimized to not copy if there's no preceding whitespace
>and p == q:
>
>From: Borislav Petkov <bp@suse.de>
>Date: Tue, 26 May 2015 10:28:17 +0200
>Subject: [PATCH] x86/cpu: Trim model id whitespace
>
>We did try trimming whitespace surrounding the 'model name' field
>in /proc/cpuinfo since reportedly some userspace uses it in string
>comparisons and there were discrepancies:
>
>[thetango@prarit ~]# grep "^model name" /proc/cpuinfo | uniq -c | sed
>'s/\ /_/g'
> ______1_model_name :_AMD_Opteron(TM)_Processor_6272
>_____63_model_name
>:_AMD_Opteron(TM)_Processor_6272_________________
>
>However, there were issues with overlapping buffers, string sizes and
>non-byte-sized copies in the previous proposed solutions; see Link tags
>below for the whole farce.
>
>So, instead of diddling with this more, let's simply extend what was
>there originally with trimming any present trailing whitespace. Final
>result is really simple and obvious.
>
>Testing with the most insane model IDs qemu can generate, looks good:
>
> .model_id = " My funny model ID CPU ",
> ______4_model_name :_My_funny_model_ID_CPU
>
> .model_id = "My funny model ID CPU ",
> ______4_model_name :_My_funny_model_ID_CPU
>
> .model_id = " My funny model ID CPU",
> ______4_model_name :_My_funny_model_ID_CPU
>
> .model_id = " ",
> ______4_model_name :__
>
> .model_id = "",
> ______4_model_name :_15/02
>
>Cc: Andy Lutomirski <luto@amacapital.net>
>Cc: Brian Gerst <brgerst@gmail.com>
>Cc: Dave Hansen <dave.hansen@linux.intel.com>
>Cc: Denys Vlasenko <dvlasenk@redhat.com>
>Cc: Fenghua Yu <fenghua.yu@intel.com>
>Cc: H. Peter Anvin <hpa@zytor.com>
>Cc: Igor Mammedov <imammedo@redhat.com>
>Cc: Linus Torvalds <torvalds@linux-foundation.org>
>Cc: Peter Zijlstra <peterz@infradead.org>
>Cc: Thomas Gleixner <tglx@linutronix.de>
>Link:
>http://lkml.kernel.org/r/1432050210-32036-1-git-send-email-prarit@redhat.com
>Link:
>http://lkml.kernel.org/r/1432628901-18044-15-git-send-email-bp@alien8.de
>Signed-off-by: Borislav Petkov <bp@suse.de>
>---
> arch/x86/kernel/cpu/common.c | 27 ++++++++++++++++++++++-----
> 1 file changed, 22 insertions(+), 5 deletions(-)
>
>diff --git a/arch/x86/kernel/cpu/common.c
>b/arch/x86/kernel/cpu/common.c
>index 41a8e9cb30bc..18120a33a2c1 100644
>--- a/arch/x86/kernel/cpu/common.c
>+++ b/arch/x86/kernel/cpu/common.c
>@@ -5,6 +5,7 @@
> #include <linux/module.h>
> #include <linux/percpu.h>
> #include <linux/string.h>
>+#include <linux/ctype.h>
> #include <linux/delay.h>
> #include <linux/sched.h>
> #include <linux/init.h>
>@@ -419,6 +420,7 @@ static const struct cpu_dev
>*cpu_devs[X86_VENDOR_NUM] = {};
> static void get_model_name(struct cpuinfo_x86 *c)
> {
> unsigned int *v;
>+ char *p, *q, *s;
>
> if (c->extended_cpuid_level < 0x80000004)
> return;
>@@ -429,11 +431,26 @@ static void get_model_name(struct cpuinfo_x86 *c)
> cpuid(0x80000004, &v[8], &v[9], &v[10], &v[11]);
> c->x86_model_id[48] = 0;
>
>- /*
>- * Remove leading whitespace on Intel processors and trailing
>- * whitespace on AMD processors.
>- */
>- memmove(c->x86_model_id, strim(c->x86_model_id), 48);
>+ /* Trim whitespace */
>+ p = q = s = &c->x86_model_id[0];
>+
>+ while (*p == ' ')
>+ p++;
>+
>+ while (*p) {
>+ /* Note the last non-whitespace index: */
>+ if (!isspace(*p))
>+ s = q;
>+
>+ /* Only copy if p advanced due to whitespace: */
>+ if (p != q)
>+ *q = *p;
>+
>+ p++;
>+ q++;
>+ }
>+
>+ *(s + 1) = '\0';
> }
>
> void cpu_detect_cache_sizes(struct cpuinfo_x86 *c)

--
Sent from my mobile phone. Please pardon brevity and lack of formatting.