On Wed, Jul 23, 2014 at 05:10:48PM -0300, Henrique de Moraes Holschuh wrote: > We must make sure the microcode has a known header format before > we attempt to access its Total Size or Data Size fields through > get_totalsize() or get_datasize().
Well, this is a pretty weak check but I'm all for being as defensive as possible with the microcode loader so yes, let's do it. However, I'd carve it out from microcode_sanity_check() in a separate function called static bool microcode_check_header(struct microcode_header_intel *hdr); and do the ->hdrver and -> ldrver checks in it. In two places to be exact... > Signed-off-by: Henrique de Moraes Holschuh <[email protected]> > --- > arch/x86/kernel/cpu/microcode/intel.c | 5 +++++ > arch/x86/kernel/cpu/microcode/intel_early.c | 3 +++ > arch/x86/kernel/cpu/microcode/intel_lib.c | 11 ++++++----- > 3 files changed, 14 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kernel/cpu/microcode/intel.c > b/arch/x86/kernel/cpu/microcode/intel.c > index a51cb19..61d430e 100644 > --- a/arch/x86/kernel/cpu/microcode/intel.c > +++ b/arch/x86/kernel/cpu/microcode/intel.c > @@ -199,6 +199,11 @@ static enum ucode_state generic_load_microcode(int cpu, > void *data, size_t size, > if (get_ucode_data(&mc_header, ucode_ptr, sizeof(mc_header))) > break; > > + if (mc_header.hdrver != 1) { > + pr_err("error! Unknown microcode update format\n"); > + break; > + } ... here ... > + > mc_size = get_totalsize(&mc_header); > if (!mc_size || mc_size > leftover) { > pr_err("error! Bad data in microcode data file\n"); > diff --git a/arch/x86/kernel/cpu/microcode/intel_early.c > b/arch/x86/kernel/cpu/microcode/intel_early.c > index b88343f..c1bf915f 100644 > --- a/arch/x86/kernel/cpu/microcode/intel_early.c > +++ b/arch/x86/kernel/cpu/microcode/intel_early.c > @@ -324,6 +324,9 @@ get_matching_model_microcode(int cpu, unsigned long start, > while (leftover) { > mc_header = (struct microcode_header_intel *)ucode_ptr; > > + if (mc_header->hdrver != 1) > + break; > + ... and here. I.e., everytime we're looking at a mc_header from userspace. > mc_size = get_totalsize(mc_header); > if (!mc_size || mc_size > leftover || > microcode_sanity_check(ucode_ptr, 0) < 0) > diff --git a/arch/x86/kernel/cpu/microcode/intel_lib.c > b/arch/x86/kernel/cpu/microcode/intel_lib.c > index ce69320..95c2d19 100644 > --- a/arch/x86/kernel/cpu/microcode/intel_lib.c > +++ b/arch/x86/kernel/cpu/microcode/intel_lib.c > @@ -52,6 +52,12 @@ int microcode_sanity_check(void *mc, int print_err) > int sum, orig_sum, ext_sigcount = 0, i; > struct extended_signature *ext_sig; > > + if (mc_header->ldrver != 1 || mc_header->hdrver != 1) { > + if (print_err) > + pr_err("error! Unknown microcode update format\n"); > + return -EINVAL; > + } > + > total_size = get_totalsize(mc_header); > data_size = get_datasize(mc_header); > > @@ -61,11 +67,6 @@ int microcode_sanity_check(void *mc, int print_err) > return -EINVAL; > } > > - if (mc_header->ldrver != 1 || mc_header->hdrver != 1) { > - if (print_err) > - pr_err("error! Unknown microcode update format\n"); > - return -EINVAL; > - } This one is then redundant. Thanks. -- 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
