linux-kernel - Re: Missing "unregister_cpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20101124090228.GA17766@liondog.tnic>
Date:	Wed, 24 Nov 2010 10:02:28 +0100
From:	Borislav Petkov <bp@...en8.de>
To:	Dave Jones <davej@...hat.com>, Neil Brown <neilb@...e.de>,
	x86@...nel.org, cpufreq@...r.kernel.org,
	linux-kernel@...r.kernel.org
Cc:	borislav.petkov@....com
Subject: Re: Missing "unregister_cpu_notifier" in powernow-k8.c

On Tue, Nov 23, 2010 at 09:31:24PM -0500, Dave Jones wrote:
> On Tue, Nov 23, 2010 at 09:19:38PM -0500, Dave Jones wrote:
>  >  The following might be a partial fix, but I suspect there is probably other
>  >  clean-up that is needed.
> 
> Indeed, looks like another possible leak in the unlikely event that the
> percpu msr struct can't be allocated..
> Instead of adding an unregister on fail, I think we can just move the registration later.

Oops, that would be partly my fault, sorry. How about the following
instead, with proper exit path labels as it is done in other kernel
code?

  [ It is not yet tested but I'll give it a run later on my X6 to verify ]


Thanks.
--
From: Borislav Petkov <borislav.petkov@....com>
Date: Wed, 24 Nov 2010 09:51:09 +0100
Subject: [PATCH] x86, powernow-k8: Fix exit path

When Cool'n'Quiet is disabled in the BIOS we're missing _PSS
objects and powernow-k8 fails loading with a juicy firmware bug
message. However, on machines with boosting cpus, it forgets
to unreg from the cpu notifier chain leading to oops. See also
https://bugzilla.novell.com/show_bug.cgi?id=655215

Fix error path accordingly.

Originally-by: Neil Brown <neilb@...e.de>
Signed-off-by: Borislav Petkov <borislav.petkov@....com>
---
 arch/x86/kernel/cpu/cpufreq/powernow-k8.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
index 491977b..bdeb5f9 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
@@ -1537,6 +1537,7 @@ static struct notifier_block cpb_nb = {
 static int __cpuinit powernowk8_init(void)
 {
 	unsigned int i, supported_cpus = 0, cpu;
+	int err = 0;
 
 	for_each_online_cpu(i) {
 		int rc;
@@ -1557,10 +1558,11 @@ static int __cpuinit powernowk8_init(void)
 
 		register_cpu_notifier(&cpb_nb);
 
+		err = -ENOMEM;
 		msrs = msrs_alloc();
 		if (!msrs) {
 			printk(KERN_ERR "%s: Error allocating msrs!\n", __func__);
-			return -ENOMEM;
+			goto unreg;
 		}
 
 		rdmsr_on_cpus(cpu_online_mask, MSR_K7_HWCR, msrs);
@@ -1574,7 +1576,15 @@ static int __cpuinit powernowk8_init(void)
 			(cpb_enabled ? "on" : "off"));
 	}
 
-	return cpufreq_register_driver(&cpufreq_amd64_driver);
+	err = cpufreq_register_driver(&cpufreq_amd64_driver);
+	if (!err)
+		goto out;
+
+unreg:
+	if (cpb_capable)
+		unregister_cpu_notifier(&cpb_nb);
+out:
+	return err;
 }
 
 /* driver entry point for term */
@@ -1582,7 +1592,7 @@ static void __exit powernowk8_exit(void)
 {
 	dprintk("exit\n");
 
-	if (boot_cpu_has(X86_FEATURE_CPB)) {
+	if (cpb_capable) {
 		msrs_free(msrs);
 		msrs = NULL;
 
-- 
1.7.2.3



-- 
Regards/Gruss,
    Boris.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/