diff mbox series

[v8,04/16] microcode/amd: fix memory leak

Message ID 1564654971-31328-5-git-send-email-chao.gao@intel.com (mailing list archive)
State Superseded
Headers show
Series improve late microcode loading | expand

Commit Message

Chao Gao Aug. 1, 2019, 10:22 a.m. UTC
Two buffers, '->equiv_cpu_table' and '->mpb',  inside 'mc_amd' might be
allocated and in the error-handing path they are not freed properly.

Signed-off-by: Chao Gao <chao.gao@intel.com>
---
changes in v8:
 - new
 - it is found by reading code. No test is done.
---
 xen/arch/x86/microcode_amd.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Jan Beulich Aug. 2, 2019, 1:39 p.m. UTC | #1
On 01.08.2019 12:22, Chao Gao wrote:
> --- a/xen/arch/x86/microcode_amd.c
> +++ b/xen/arch/x86/microcode_amd.c
> @@ -433,6 +433,9 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf,
>           goto out;
>       }
>   
> +    mc_amd->equiv_cpu_table_size = 0;
> +    mc_amd->equiv_cpu_table = NULL;

Instead of adding these, you could as well use xzalloc()
further up and drop the explicit initialization of ->mpb and
->mpb_size to NULL/0 a few lines down.

> @@ -479,6 +482,8 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf,
>   
>       if ( error )
>       {
> +        if ( mc_amd->equiv_cpu_table_size )
> +            xfree(mc_amd->equiv_cpu_table);

Why the if()? There's no problem calling xfree() with a NULL
argument.

> @@ -549,11 +554,14 @@ static int cpu_request_microcode(unsigned int cpu, const void *buf,
>   
>       if ( save_error )
>       {
> -        xfree(mc_amd);
>           uci->mc.mc_amd = mc_old;
> +        mc_old = mc_amd;
>       }
> -    else
> -        xfree(mc_old);
> +
> +    if ( mc_old->mpb_size )
> +        xfree(mc_old->mpb);
> +    xfree(mc_old->equiv_cpu_table);

Same here. With the adjustments made (could possibly be done
again while committing)
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index 7a854c0..afca51f 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -433,6 +433,9 @@  static int cpu_request_microcode(unsigned int cpu, const void *buf,
         goto out;
     }
 
+    mc_amd->equiv_cpu_table_size = 0;
+    mc_amd->equiv_cpu_table = NULL;
+
     /*
      * Multiple container file support:
      * 1. check if this container file has equiv_cpu_id match
@@ -479,6 +482,8 @@  static int cpu_request_microcode(unsigned int cpu, const void *buf,
 
     if ( error )
     {
+        if ( mc_amd->equiv_cpu_table_size )
+            xfree(mc_amd->equiv_cpu_table);
         xfree(mc_amd);
         goto out;
     }
@@ -549,11 +554,14 @@  static int cpu_request_microcode(unsigned int cpu, const void *buf,
 
     if ( save_error )
     {
-        xfree(mc_amd);
         uci->mc.mc_amd = mc_old;
+        mc_old = mc_amd;
     }
-    else
-        xfree(mc_old);
+
+    if ( mc_old->mpb_size )
+        xfree(mc_old->mpb);
+    xfree(mc_old->equiv_cpu_table);
+    xfree(mc_old);
 
   out:
 #if CONFIG_HVM