lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 30 Sep 2014 22:28:52 +0200 (CEST) From: Thomas Gleixner <tglx@...utronix.de> To: Bryan O'Donoghue <pure.logic@...us-software.ie> cc: mingo@...hat.com, davej@...hat.com, hpa@...or.com, hmh@....eng.br, x86@...nel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH 1/3] x86: Bugfix bit-rot in the calling of legacy_cache_size On Tue, 30 Sep 2014, Bryan O'Donoghue wrote: > static void default_init(struct cpuinfo_x86 *c) > { > -#ifdef CONFIG_X86_64 > cpu_detect_cache_sizes(c); > -#else > + > /* Not much we can do here... */ > /* Check if at least it has cpuid */ > if (c->cpuid_level == -1) { > @@ -79,7 +78,6 @@ static void default_init(struct cpuinfo_x86 *c) > else if (c->x86 == 3) > strcpy(c->x86_model_id, "386"); > } > -#endif What's the exact point of this? This is the default init function which is for X86_VENDOR_UNKNOWN, right? > } > > static const struct cpu_dev default_cpu = { > @@ -874,6 +872,15 @@ static void identify_cpu(struct cpuinfo_x86 *c) > #endif > > /* > + * If c_x86_vendor != X86_VENDOR_UNKNOWN i.e. a known vendor then > + * there's a vendor specific c_init() > + * > + * Even still Intel, AMD and VIA make use of legacy_cache_size which > + * is reachable only through default_init right now That's nonsense. It's called from cpu_detect_cache_sizes() and that is called from: arch/x86/kernel/cpu/amd.c: cpu_detect_cache_sizes(c); arch/x86/kernel/cpu/centaur.c: cpu_detect_cache_sizes(c); arch/x86/kernel/cpu/common.c: cpu_detect_cache_sizes(c); arch/x86/kernel/cpu/cyrix.c: cpu_detect_cache_sizes(c); arch/x86/kernel/cpu/transmeta.c: cpu_detect_cache_sizes(c); So we have 4 callsites outside of default_init() already. Now you're adding another one into identify_cpu() > + */ > + if (this_cpu->c_x86_vendor != X86_VENDOR_UNKNOWN) > + default_init(c); With the consequence that for amd/centaur/cyrix/transmeta cpu_detect_cache_sizes() is invoked twice for nothing. So the only CPU vendor c_init() callback which does not call cpu_detect_cache_sizes() is the Intel one. And therefor we inflict that call on all others twice. Brilliant solution! So the proper solution is to add the call to identify_cpu() unconditionally and remove it from _all_ other call sites, make it static and be done with it. Sigh, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists