Message ID | 20241028091856.2151603-11-andrew.cooper3@citrix.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | x86/ucode: Fix module-handling use-aftere-free's | expand |
On 28.10.2024 10:18, Andrew Cooper wrote: > The data pointer is known good, so the -ENOMEM path is unnecessary. However, > if we find no patch, something's wrong seeing as early_microcode_init() > indicated it was happy. > > We are the entity initialising the cache, so we don't need the complexity of > using microcode_update_cache(). Just assert the cache is empty, and populate > it. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
On 10/28/24 05:18, Andrew Cooper wrote: > The data pointer is known good, so the -ENOMEM path is unnecessary. However, > if we find no patch, something's wrong seeing as early_microcode_init() > indicated it was happy. > > We are the entity initialising the cache, so we don't need the complexity of > using microcode_update_cache(). Just assert the cache is empty, and populate > it. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Daniel P. Smith <dpsmith@apertussolutions.com> > --- Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c index 591c13ad91fb..90a6f2bd87ae 100644 --- a/xen/arch/x86/cpu/microcode/core.c +++ b/xen/arch/x86/cpu/microcode/core.c @@ -707,33 +707,6 @@ int microcode_update_one(void) return microcode_update_cpu(NULL, 0); } -static int __init early_update_cache(const void *data, size_t len) -{ - int rc = 0; - struct microcode_patch *patch; - - if ( !data ) - return -ENOMEM; - - patch = parse_blob(data, len); - if ( IS_ERR(patch) ) - { - printk(XENLOG_WARNING "Parsing microcode blob error %ld\n", - PTR_ERR(patch)); - return PTR_ERR(patch); - } - - if ( !patch ) - return -ENOENT; - - spin_lock(µcode_mutex); - rc = microcode_update_cache(patch); - spin_unlock(µcode_mutex); - ASSERT(rc); - - return rc; -} - /* * Set by early_microcode_load() to indicate where it found microcode, so * microcode_init_cache() can find it again and initalise the cache. opt_scan @@ -744,6 +717,7 @@ static int __initdata early_mod_idx = -1; static int __init cf_check microcode_init_cache(void) { struct boot_info *bi = &xen_boot_info; + struct microcode_patch *patch; void *data; size_t size; int rc = 0; @@ -774,7 +748,24 @@ static int __init cf_check microcode_init_cache(void) size = cd.size; } - rc = early_update_cache(data, size); + patch = parse_blob(data, size); + if ( IS_ERR(patch) ) + { + rc = PTR_ERR(patch); + printk(XENLOG_WARNING "Microcode: Parse error %d\n", rc); + return rc; + } + + if ( !patch ) + { + printk(XENLOG_WARNING "Microcode: No suitable patch found\n"); + return -ENOENT; + } + + spin_lock(µcode_mutex); + ASSERT(microcode_cache == NULL); + microcode_cache = patch; + spin_unlock(µcode_mutex); return rc; }
The data pointer is known good, so the -ENOMEM path is unnecessary. However, if we find no patch, something's wrong seeing as early_microcode_init() indicated it was happy. We are the entity initialising the cache, so we don't need the complexity of using microcode_update_cache(). Just assert the cache is empty, and populate it. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Daniel P. Smith <dpsmith@apertussolutions.com> --- xen/arch/x86/cpu/microcode/core.c | 47 +++++++++++++------------------ 1 file changed, 19 insertions(+), 28 deletions(-)