On Mon, Sep 08, 2014 at 02:37:47PM -0300, Henrique de Moraes Holschuh wrote:
> The Intel SDM vol 3A, section 9.11.1, and also table 9-6, requires that
> the Data Size field be a multiple of 4 bytes. All of the microcode
> update header structures are dword-based, so the Total Size field must
> also be a multiple of the dword size.
>
> Ensure that data_size is a multiple of the dword size (4 bytes). The
> driver code assumes this to be true for both data_size and total_size,
> and will not work correctly otherwise.
>
> Futhermore, require that total_size be a multiple of 1024, as per the
> Intel SDM, vol 3A, section 9.11.1, page 9-28; table 9-6, page 9-29, and
> others. Test added by request of Borislav Petkov.
>
> Also refuse a microcode update with a microcode revision of zero.
> According to the Intel SDM, vol 3A, section 9.11.7, page 9-36, a
> microcode revision of zero is special:
>
> "CPUID returns a value in a model specific register in addition to
> its usual register return values. The semantics of CPUID cause it
> to deposit an update ID value in the 64-bit model-specific register
> at address 08BH (IA32_BIOS_SIGN_ID). If no update is present in the
> processor, the value in the MSR remains unmodified. The BIOS must
> pre-load a zero into the MSR before executing CPUID. If a read of
> the MSR at 8BH still returns zero after executing CPUID, this
> indicates that no update is present."
>
> This effectively reserves revision zero to mean "no microcode update
> installed on the processor": the microcode loader cannot differentiate
> sucess from failure when updating microcode to the same revision as the
> one currently installed on the processor, and this would always happen
> to updates to revision zero in the BIOS/UEFI loader.
>
> There is every reason to be paranoid about any microcode update with a
> revision of zero, as Intel will never release such a microcode update.
>
> Signed-off-by: Henrique de Moraes Holschuh <h...@hmh.eng.br>
> ---
> arch/x86/kernel/cpu/microcode/intel_lib.c | 25 +++++++++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/microcode/intel_lib.c
> b/arch/x86/kernel/cpu/microcode/intel_lib.c
> index ce69320..25915e3 100644
> --- a/arch/x86/kernel/cpu/microcode/intel_lib.c
> +++ b/arch/x86/kernel/cpu/microcode/intel_lib.c
> @@ -55,9 +55,10 @@ int microcode_sanity_check(void *mc, int print_err)
> total_size = get_totalsize(mc_header);
> data_size = get_datasize(mc_header);
>
> - if (data_size + MC_HEADER_SIZE > total_size) {
> + if ((data_size % DWSIZE) || (total_size % 1024) ||
> + (data_size + MC_HEADER_SIZE > total_size)) {
> if (print_err)
> - pr_err("error! Bad data size in microcode data file\n");
> + pr_err("error: bad data size or total size in microcode
> data file\n");
Shorten:
pr_err("error: bad data/total size in microcode data file\n");
> return -EINVAL;
> }
>
> @@ -83,6 +84,26 @@ int microcode_sanity_check(void *mc, int print_err)
> ext_sigcount = ext_header->count;
> }
>
> + /*
> + * A version 1 loader cannot differentiate failure from success when
> + * attempting a microcode update to the same revision as the one
> + * currently installed. The loader is supposed to never attempt a
> + * same-version update (or a microcode downgrade, for that matter).
> + *
> + * This will always cause issues for microcode updates to revision zero
> + * in the UEFI/BIOS microcode loader: the processor reports a revision
> + * of zero when it is running without any microcode updates installed,
> + * such as after a reset/power up.
> + *
> + * Intel will never issue a microcode update with a revision of zero
> + * for the version 1 loader. Reject it.
> + */
This comment is too long. How about this instead:
/*
* 0 is not a valid microcode revision as it is used to denote the
* failure of a microcode update, see MSR 0x8b (IA32_BIOS_SIGN_ID):
*
* "It is required that this register field be pre-loaded with zero
* prior to executing the CPUID, function 1. If the field remains
* equal to zero, then there is no microcode update loaded. Another
* non-zero value will be the signature."
*/
This is one of those seldom times where the documentation is actually clear. :-)
--
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
