linux-kernel - Re: [PATCH] x86: Add check for P5 to microcode_intel

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5171AA86.604@nexus-software.ie>
Date:	Fri, 19 Apr 2013 21:35:18 +0100
From:	Bryan O'Donoghue <bryan.odonoghue.lkml@...us-software.ie>
To:	Borislav Petkov <bp@...en8.de>
CC:	hpa@...or.com, linux-kernel@...r.kernel.org, x86@...nel.org,
	fenghua.yu@...el.com
Subject: Re: [PATCH] x86: Add check for P5 to microcode_intel_early

On 19/04/13 20:11, Borislav Petkov wrote:
> On Fri, Apr 19, 2013 at 06:23:03PM +0100, Bryan O'Donoghue wrote:
>> Architectural MSRs associated with microcode are for P6 or higher.
>> Add a check to early microcode to detect<  P6.
>>
>> Without a check for<  P6 - we end up reading from unimplemented MSRs
>> on Pentium.
>
> Is this something you're actually seeing on some box or just found by
> staring at the code?

We actually see this on a P5 alright.
The code path is reachable, no question.

> In any case, the family checks should go into the ucode driver entry
> points in arch/x86/kernel/microcode_core_early.c. AFAICT, x86_vendor()
> is a good candidate to be taught to read out the family too and return
> X86_VENDOR_UNKNOWN if<  P6. Or something to that effect.

Hmm

Just returning X86_VENDOR_UNKNOWN - won't fix the bug though - after all 
MSR_IA32_UCODE_REV is also x86_family() >= 6

x86 = get_x86_family(csig.sig);
x86_model = get_x86_model(csig.sig);

if (x86 < 6)
	return UCODE_ERROR;

if ((x86_model >= 5) || (x86 > 6)) {
	/* get processor flags from MSR 0x17 */
	native_rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]);
	csig.pf = 1 << ((val[1] >> 18) & 7);
}

native_wrmsr(MSR_IA32_UCODE_REV, 0, 0);

/* As documented in the SDM: Do a CPUID 1 here */
sync_core();

/* get the current revision from MSR 0x8B */
native_rdmsr(MSR_IA32_UCODE_REV, val[0], val[1]);

Peter - what's your take ?

--
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/