[PATCH]: ACPI: Add the module param check for processor.max_cstate

[PATCH]: ACPI: Add the module param check for processor.max_cstate

[yakui_zhao]

From: Zhao Yakui <yakui.zhao@intel.com>

When the boot option of "processor.max_cstate=0" is added, the C-state 
won't be configured correctly. In such case the cpu idle function will access
the uninitialized memory region and then kernel panic will happen.

So it is necessary to add the module param check for the processor.max_cstate.
If it is equal to or less than zero, it will be ignored. If it is beyond the
max threshold, it is also ignored.

http://bugzilla.kernel.org/show_bug.cgi?id=13142

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
 drivers/acpi/processor_idle.c |   27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

Index: linux-2.6/drivers/acpi/processor_idle.c
===================================================================
--- linux-2.6.orig/drivers/acpi/processor_idle.c	2009-04-21 15:48:52.000000000 +0800
+++ linux-2.6/drivers/acpi/processor_idle.c	2009-04-24 10:00:47.000000000 +0800
@@ -70,7 +70,32 @@
 #define PM_TIMER_TICKS_TO_US(p)		(((p) * 1000)/(PM_TIMER_FREQUENCY/1000))
 
 static unsigned int max_cstate __read_mostly = ACPI_PROCESSOR_MAX_POWER;
-module_param(max_cstate, uint, 0000);
+
+static int param_set_max_cstate(const char *val, struct kernel_param *kp)
+{
+	unsigned long cstate = 0;
+	int ret = 0;
+
+	if (!val)
+		return -EINVAL;
+
+	ret = strict_strtoul(val, 10, &cstate);
+	if (ret == -EINVAL || (unsigned int)cstate != cstate)
+		return -EINVAL;
+	/*
+	 * if the value is equal to or less than zero, it will be ingored.
+	 * Of course it will also be ignored if it is bigger than the
+	 * threshold of ACPI_PROCESSOR_MAX_POWER.
+	 */
+	if (cstate <= 0 || cstate > ACPI_PROCESSOR_MAX_POWER)
+		return -EINVAL;
+
+	max_cstate = cstate;
+
+	return 0;
+}
+module_param_call(max_cstate, param_set_max_cstate, NULL,
+		  &max_cstate, 0000);
 static unsigned int nocst __read_mostly;
 module_param(nocst, uint, 0000);
 

Re: [PATCH]: ACPI: Add the module param check for processor.max_cstate

[Len Brown]

From: Len Brown <len.brown@intel.com>
Subject: [PATCH] ACPI: prevent processor.max_cstate=0 boot crash

As processor.max_cstate is an init-time-only modparam,
sanity checking it at init-time is sufficient.

http://bugzilla.kernel.org/show_bug.cgi?id=13142

Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/processor_idle.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 6fe1214..436127e 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1037,6 +1037,9 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
 		dev->states[i].desc[0] = '\0';
 	}
 
+	if (max_cstate == 0)
+		max_cstate = 1;
+
 	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER && i <= max_cstate; i++) {
 		cx = &pr->power.states[i];
 		state = &dev->states[count];
-- 
1.6.3.rc1.34.g0be9b

Re: [PATCH]: ACPI: Add the module param check for processor.max_cstate

[yakui_zhao]

On Fri, 2009-04-24 at 11:24 +0800, Len Brown wrote:
> From: Len Brown <len.brown@intel.com>
> Subject: [PATCH] ACPI: prevent processor.max_cstate=0 boot crash
> 
> As processor.max_cstate is an init-time-only modparam,
> sanity checking it at init-time is sufficient.
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=13142
This is also OK. And it is easier.

Thanks.
> 
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
>  drivers/acpi/processor_idle.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 6fe1214..436127e 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -1037,6 +1037,9 @@ static int acpi_processor_setup_cpuidle(struct acpi_processor *pr)
>  		dev->states[i].desc[0] = '\0';
>  	}
>  
> +	if (max_cstate == 0)
> +		max_cstate = 1;
> +
>  	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER && i <= max_cstate; i++) {
>  		cx = &pr->power.states[i];
>  		state = &dev->states[count];