RE: [EXTERNAL] [PATCH] x86/MCE/AMD: Fix use after free in error handling

[Saar Amar <Saar.Amar@microsoft.com>]

That's correct - I found this issue through reading and audit the code statically. Thanks for helping out Dan!

-----Original Message-----
From: Dan Carpenter <dan.carpenter@oracle.com> 
Sent: Tuesday, January 28, 2020 4:10 PM
To: Tony Luck <tony.luck@intel.com>; Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; H. Peter Anvin <hpa@zytor.com>; x86@kernel.org; linux-edac@vger.kernel.org; Saar Amar <Saar.Amar@microsoft.com>; security@kernel.org
Subject: [EXTERNAL] [PATCH] x86/MCE/AMD: Fix use after free in error handling

If an error occurs in the threshold_create_bank() function then the real clean up is supposed to happen in mce_threshold_remove_device().  The problem here is that if allocate_threshold_blocks() fails, then we
kfree(b) before returning.  Then we use "b" again in
mce_threshold_remove_device() when we do the rest of the clean up work.

Fixes: 019f34fccfd5 ("x86, MCE, AMD: Move shared bank to node descriptor")
Reported-by: Saar Amar <Saar.Amar@microsoft.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
I believe Saar found this through reading the code and there is no test case.  I have don't have a way to test it.

 arch/x86/kernel/cpu/mce/amd.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/mce/amd.c b/arch/x86/kernel/cpu/mce/amd.c index b3a50d962851..ff01b789066e 100644
--- a/arch/x86/kernel/cpu/mce/amd.c
+++ b/arch/x86/kernel/cpu/mce/amd.c
@@ -1342,7 +1342,8 @@ static int threshold_create_bank(unsigned int cpu, unsigned int bank)
 	b->kobj = kobject_create_and_add(name, &dev->kobj);
 	if (!b->kobj) {
 		err = -EINVAL;
-		goto out_free;
+		kfree(b);
+		goto out;
 	}
 
 	per_cpu(threshold_banks, cpu)[bank] = b; @@ -1358,12 +1359,6 @@ static int threshold_create_bank(unsigned int cpu, unsigned int bank)
 	}
 
 	err = allocate_threshold_blocks(cpu, bank, 0, msr_ops.misc(bank));
-	if (!err)
-		goto out;
-
- out_free:
-	kfree(b);
-
  out:
 	return err;
 }
--
2.11.0