linux-kernel - Re: [PATCH 2/2] x86/microcode: Rework early revisions reporting

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: <ZWpDHgNjhQKLodF6@a4bf019067fa.jf.intel.com>
Date:   Fri, 1 Dec 2023 12:33:34 -0800
From:   Ashok Raj <ashok.raj@...el.com>
To:     Borislav Petkov <bp@...en8.de>
CC:     X86 ML <x86@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
        "Linus Torvalds" <torvalds@...ux-foundation.org>,
        Ashok Raj <ashok.raj@...el.com>
Subject: Re: [PATCH 2/2] x86/microcode: Rework early revisions reporting

On Fri, Dec 01, 2023 at 05:39:28PM +0100, Borislav Petkov wrote:
> On Thu, Nov 30, 2023 at 10:34:31AM -0800, Ashok Raj wrote:
> > new_rev is always assigned even if there was no microcode to apply.
> 
> That is wrong.
> 
> In my defence, I don't have an Intel machine which has up-to-date
> microcode in the BIOS and the blob has the same microcode revision. All
> my Intel test boxes do update microcode.
> 
> > - On AMD, the ucode is loaded even if the current revision matches what is
> >   being loaded.
> 
> From the commit message of the patch you're replying to:
> 
> "What is also missing on the AMD side - something which people have
> requested before - is showing the microcode revision the CPU had
> *before* the early update."
> 
> The logic is, we want to dump before-after everytime there was
> a successful early upload.
> 
> > Currently, it's displayed as below:
> > 
> > [  113.395868] microcode: Current revision: 0x21000170
> > [  113.404244] microcode: Updated early from: 0x21000170
> 
> There's something else weird going on though. I would expect that that
> machine should not update microcode if it cannot find newer. Maybe the
> scan_microcode() logic is a bit weird still.
> 
> Please run this debug + fix patch and send me full dmesg from that
> machine.
> 

I'll get a dmesg shortly once i get my test system back.

What I meant was 

void __init load_ucode_intel_bsp(struct early_load_data *ed)
{
        struct ucode_cpu_info uci;

        ed->old_rev = intel_get_microcode_revision();

        uci.mc = get_microcode_blob(&uci, false);
        if (uci.mc && apply_microcode_early(&uci) == UCODE_UPDATED)
                ucode_patch_va = UCODE_BSP_LOADED;

        ed->new_rev = uci.cpu_sig.rev;
}

ed->new_rev is always assigned, just not for the UCODE_UPDATED case. Hence
even if we had no microcode to update ed->new_rev ends up being non-zero.