linux-kernel - Re: [patch V3 30/30] x86/microcode/intel: Add a minimum required revision for late-loads

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: <20230925162032.56408-1-qiuxu.zhuo@intel.com>
Date:   Tue, 26 Sep 2023 00:20:32 +0800
From:   Qiuxu Zhuo <qiuxu.zhuo@...el.com>
To:     tglx@...utronix.de
Cc:     arjan@...ux.intel.com, ashok.raj@...el.com, bp@...en8.de,
        chang.seok.bae@...el.com, linux-kernel@...r.kernel.org,
        nik.borisov@...e.com, x86@...nel.org, qiuxu.zhuo@...el.com
Subject: Re: [patch V3 30/30] x86/microcode/intel: Add a minimum required revision for late-loads

> ...
> From: Ashok Raj <ashok.raj@...el.com>
> 
> In general users don't have the necessary information to determine whether
> late loading of a new microcode version is safe and does not modify
> anything which the currently running kernel uses already, e.g. removal of
> CPUID bits or behavioural changes of MSRs.
> ...
> 
> The check is always enabled, but by default not enforced. It can be
> enforced via Kconfig or kernel command line.
> 
> If enforced, the kernel refuses to late load microcode with a minium

s/minium/minimum/

> required version field which is zero or when the currently loaded microcode
> revision is smaller than the minimum required revision.
> 
> ...
> --- a/arch/x86/kernel/cpu/microcode/intel.c
> +++ b/arch/x86/kernel/cpu/microcode/intel.c
> @@ -463,16 +463,40 @@ static enum ucode_state apply_microcode_
>  	return ret;
>  }
>  
> +static bool ucode_validate_minrev(struct microcode_header_intel *mc_header)
> +{
> +	int cur_rev = boot_cpu_data.microcode;
> +
> +	/*
> +	 * When late-loading, ensure the header declares a minimum revision
> +	 * required to perform a late-load. The previously reserved field
> +	 * is 0 in older microcode blobs.
> +	 */
> +	if (!mc_header->min_req_ver) {
> +		pr_info("Unsafe microcode update: Microcode header does not specify a required min version\n");
> +		return false;
> +	}
> +
> +	/*
> +	 * Check whether the minimum revision specified in the header is either
> +	 * greater or equal to the current revision.
> +	 */

Seems like the above comment doesn't match the following 'if' check.
Perhaps the comment is:

   "Check whether the current revision is either greater or
    equal to the minimum revision specified in the header."

> +	if (cur_rev < mc_header->min_req_ver) {
> +		pr_info("Unsafe microcode update: Current revision 0x%x too old\n", cur_rev);
> +		pr_info("Current should be at 0x%x or higher. Use early loading instead\n", mc_header->min_req_ver);
> +		return false;
> +	}
> +	return true;
> +}
> ...