[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-edac
Subject:    [PATCH] x86/MCE/AMD: Fix use after free in error handling
From:       Dan Carpenter <dan.carpenter () oracle ! com>
Date:       2020-01-28 14:09:52
Message-ID: 20200128140846.phctkvx5btiexvbx () kili ! mountain
[Download RAW message or body]

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

[prev in list] [next in list] [prev in thread] [next in thread]