From a99f03428faa2e53cce17aec0d636de760ec281a Mon Sep 17 00:00:00 2001 From: Shu Wang Date: Mon, 24 Jul 2017 12:12:26 +0200 Subject: x86/microcode/AMD: Free unneeded patch before exit from update_cache() verify_and_add_patch() allocates memory for a microcode patch and hands it down to be added to the cache of patches. However, if the cache already has the latest patch, the newly allocated one needs to be freed before returning. Do that. This issue has been found by kmemleak: unreferenced object 0xffff88010e780b40 (size 32): comm "bash", pid 860, jiffies 4294690939 (age 29.297s) backtrace: kmemleak_alloc kmem_cache_alloc_trace load_microcode_amd.isra.0 request_microcode_amd reload_store dev_attr_store sysfs_kf_write kernfs_fop_write __vfs_write vfs_write SyS_write do_syscall_64 return_from_SYSCALL_64 0xffffffffffffffff (gdb) list *0xffffffff81050d60 0xffffffff81050d60 is in load_microcode_amd (arch/x86/kernel/cpu/microcode/amd.c:616). which is this: patch = kzalloc(sizeof(*patch), GFP_KERNEL); --> if (!patch) { pr_err("Patch allocation failure.\n"); return -EINVAL; } Signed-off-by: Shu Wang [ Rewrite commit message. ] Signed-off-by: Borislav Petkov Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: chuhu@redhat.com Cc: liwang@redhat.com Link: http://lkml.kernel.org/r/20170724101228.17326-2-bp@alien8.de Signed-off-by: Ingo Molnar --- arch/x86/kernel/cpu/microcode/amd.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'arch') diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c index 21b185793c80..c6daec4bdba5 100644 --- a/arch/x86/kernel/cpu/microcode/amd.c +++ b/arch/x86/kernel/cpu/microcode/amd.c @@ -400,9 +400,12 @@ static void update_cache(struct ucode_patch *new_patch) list_for_each_entry(p, µcode_cache, plist) { if (p->equiv_cpu == new_patch->equiv_cpu) { - if (p->patch_id >= new_patch->patch_id) + if (p->patch_id >= new_patch->patch_id) { /* we already have the latest patch */ + kfree(new_patch->data); + kfree(new_patch); return; + } list_replace(&p->plist, &new_patch->plist); kfree(p->data); -- cgit v1.2.3 From aa78c1ccfab6018289bc2bfd0092d516d0a49ec5 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Fri, 25 Aug 2017 12:04:56 +0200 Subject: x86/microcode/intel: Improve microcode patches saving flow Avoid potentially dereferencing a NULL pointer when saving a microcode patch for early loading on the application processors. While at it, drop the IS_ERR() checking in favor of simpler, NULL-ptr checks which are sufficient and rename __alloc_microcode_buf() to memdup_patch() to more precisely denote what it does. No functionality change. Reported-by: Dan Carpenter Signed-off-by: Borislav Petkov Signed-off-by: Thomas Gleixner Cc: kernel-janitors@vger.kernel.org Link: http://lkml.kernel.org/r/20170825100456.n236w3jebteokfd6@pd.tnic --- arch/x86/kernel/cpu/microcode/intel.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) (limited to 'arch') diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index 59edbe9d4ccb..8f7a9bbad514 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -146,18 +146,18 @@ static bool microcode_matches(struct microcode_header_intel *mc_header, return false; } -static struct ucode_patch *__alloc_microcode_buf(void *data, unsigned int size) +static struct ucode_patch *memdup_patch(void *data, unsigned int size) { struct ucode_patch *p; p = kzalloc(sizeof(struct ucode_patch), GFP_KERNEL); if (!p) - return ERR_PTR(-ENOMEM); + return NULL; p->data = kmemdup(data, size, GFP_KERNEL); if (!p->data) { kfree(p); - return ERR_PTR(-ENOMEM); + return NULL; } return p; @@ -183,8 +183,8 @@ static void save_microcode_patch(void *data, unsigned int size) if (mc_hdr->rev <= mc_saved_hdr->rev) continue; - p = __alloc_microcode_buf(data, size); - if (IS_ERR(p)) + p = memdup_patch(data, size); + if (!p) pr_err("Error allocating buffer %p\n", data); else list_replace(&iter->plist, &p->plist); @@ -196,24 +196,25 @@ static void save_microcode_patch(void *data, unsigned int size) * newly found. */ if (!prev_found) { - p = __alloc_microcode_buf(data, size); - if (IS_ERR(p)) + p = memdup_patch(data, size); + if (!p) pr_err("Error allocating buffer for %p\n", data); else list_add_tail(&p->plist, µcode_cache); } + if (!p) + return; + /* * Save for early loading. On 32-bit, that needs to be a physical * address as the APs are running from physical addresses, before * paging has been enabled. */ - if (p) { - if (IS_ENABLED(CONFIG_X86_32)) - intel_ucode_patch = (struct microcode_intel *)__pa_nodebug(p->data); - else - intel_ucode_patch = p->data; - } + if (IS_ENABLED(CONFIG_X86_32)) + intel_ucode_patch = (struct microcode_intel *)__pa_nodebug(p->data); + else + intel_ucode_patch = p->data; } static int microcode_sanity_check(void *mc, int print_err) -- cgit v1.2.3