Skip to contentSkip to conversation

wmi: Fix kernel panic when stack protection enabled.

PATCH In review [email protected] 2 participants 4 messages Jun 9, 2009 – Jun 18, 2009

Subsystems (2)
ACPI x86 platform drivers
lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/1 wmi: Fix kernel panic when stack protection enabled.

From: Costantino Leandro <[email protected]>

Summary:

Kernel panic arise when stack protection is enabled, since strncat will 
add a null terminating byte '\0'; So in functions
like this one (wmi_query_block):
        char wc[4]="WC";
	....
	strncat(method, block->object_id, 2);
        ...

the length of wc should be n+1 (wc[5]) or stack protection fault will arise. This is not noticeable when stack protection is disabled,but , isn't good either.
Config used: [CONFIG_CC_STACKPROTECTOR_ALL=y,
CONFIG_CC_STACKPROTECTOR=y]

Panic Trace
------------

       .... stack-protector: kernel stack corrupted in : fa7b182c
       2.6.30-rc8-obelisco-generic
       call_trace:
           [<c04a6c40>] ? panic+0x45/0xd9
	   [<c012925d>] ? __stack_chk_fail+0x1c/0x40
	   [<fa7b182c>] ? wmi_query_block+0x15a/0x162 [wmi]
	   [<fa7b182c>] ? wmi_query_block+0x15a/0x162 [wmi]
	   [<fa7e7000>] ? acer_wmi_init+0x00/0x61a [acer_wmi]
	   [<fa7e7135>] ? acer_wmi_init+0x135/0x61a [acer_wmi]
	   [<c0101159>] ? do_one_initcall+0x50+0x126
Signed-off-by Costantino Leandro <[email protected]>
---
 drivers/platform/x86/wmi.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
drivers/platform/x86/wmi.c +4 -4
index 043b208..f215a59 100644
@@ -270,7 +270,7 @@ u32 method_id, const struct acpi_buffer *in, struct acpi_buffer *out)
 	acpi_status status;
 	struct acpi_object_list input;
 	union acpi_object params[3];
-	char method[4] = "WM";
+	char method[5] = "WM";
 
 	if (!find_guid(guid_string, &wblock))
 		return AE_ERROR;

@@ -328,8 +328,8 @@ struct acpi_buffer *out)
 	acpi_status status, wc_status = AE_ERROR;
 	struct acpi_object_list input, wc_input;
 	union acpi_object wc_params[1], wq_params[1];
-	char method[4];
-	char wc_method[4] = "WC";
+	char method[5];
+	char wc_method[5] = "WC";
 
 	if (!guid_string || !out)
 		return AE_BAD_PARAMETER;

@@ -410,7 +410,7 @@ const struct acpi_buffer *in)
 	acpi_handle handle;
 	struct acpi_object_list input;
 	union acpi_object params[2];
-	char method[4] = "WS";
+	char method[5] = "WS";
 
 	if (!guid_string || !in)
 		return AE_BAD_DATA;
Signature · 1 line
1.5.6.5
Acked-by on v1 1/1
#1 lore raw

[Adding Len to CC]

Quoted 29 lines · On Tuesday 09 Jun 2009 22:09:23 [email protected] wrote:

From: Costantino Leandro <[email protected]>

Summary:

Kernel panic arise when stack protection is enabled, since strncat will
add a null terminating byte '\0'; So in functions
like this one (wmi_query_block):
        char wc[4]="WC";
	....
	strncat(method, block->object_id, 2);
        ...

the length of wc should be n+1 (wc[5]) or stack protection fault will arise. This is not noticeable when stack protection is disabled,but , isn't good either.
Config used: [CONFIG_CC_STACKPROTECTOR_ALL=y,
CONFIG_CC_STACKPROTECTOR=y]

Panic Trace
------------

       .... stack-protector: kernel stack corrupted in : fa7b182c
       2.6.30-rc8-obelisco-generic
       call_trace:
           [<c04a6c40>] ? panic+0x45/0xd9
	   [<c012925d>] ? __stack_chk_fail+0x1c/0x40
	   [<fa7b182c>] ? wmi_query_block+0x15a/0x162 [wmi]
	   [<fa7b182c>] ? wmi_query_block+0x15a/0x162 [wmi]
	   [<fa7e7000>] ? acer_wmi_init+0x00/0x61a [acer_wmi]
	   [<fa7e7135>] ? acer_wmi_init+0x135/0x61a [acer_wmi]
	   [<c0101159>] ? do_one_initcall+0x50+0x126
Quoted 1 line
Signed-off-by Costantino Leandro <[email protected]>
Acked-by Carlos Corbacho <[email protected]>

Len, can you take this one for 2.6.31?

Quoted 37 lines
---
 drivers/platform/x86/wmi.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
drivers/platform/x86/wmi.c +4 -4
index 043b208..f215a59 100644
@@ -270,7 +270,7 @@ u32 method_id, const struct acpi_buffer *in, struct
 acpi_buffer *out) acpi_status status;
 	struct acpi_object_list input;
 	union acpi_object params[3];
-	char method[4] = "WM";
+	char method[5] = "WM";
 
 	if (!find_guid(guid_string, &wblock))
 		return AE_ERROR;

@@ -328,8 +328,8 @@ struct acpi_buffer *out)
 	acpi_status status, wc_status = AE_ERROR;
 	struct acpi_object_list input, wc_input;
 	union acpi_object wc_params[1], wq_params[1];
-	char method[4];
-	char wc_method[4] = "WC";
+	char method[5];
+	char wc_method[5] = "WC";
 
 	if (!guid_string || !out)
 		return AE_BAD_PARAMETER;

@@ -410,7 +410,7 @@ const struct acpi_buffer *in)
 	acpi_handle handle;
 	struct acpi_object_list input;
 	union acpi_object params[2];
-	char method[4] = "WS";
+	char method[5] = "WS";
 
 	if (!guid_string || !in)
 		return AE_BAD_DATA;
Signature · 3 lines
E-Mail: [email protected]
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D
#2 lore raw
Quoted 32 lines · On Tuesday 09 Jun 2009 20:46:24 Carlos Corbacho wrote:

[Adding Len to CC]

Quoted 29 lines · On Tuesday 09 Jun 2009 22:09:23 [email protected] wrote:

From: Costantino Leandro <[email protected]>

Summary:

Kernel panic arise when stack protection is enabled, since strncat will
add a null terminating byte '\0'; So in functions
like this one (wmi_query_block):
        char wc[4]="WC";
	....
	strncat(method, block->object_id, 2);
        ...

the length of wc should be n+1 (wc[5]) or stack protection fault will arise. This is not noticeable when stack protection is disabled,but , isn't good either.
Config used: [CONFIG_CC_STACKPROTECTOR_ALL=y,
CONFIG_CC_STACKPROTECTOR=y]

Panic Trace
------------

       .... stack-protector: kernel stack corrupted in : fa7b182c
       2.6.30-rc8-obelisco-generic
       call_trace:
           [<c04a6c40>] ? panic+0x45/0xd9
	   [<c012925d>] ? __stack_chk_fail+0x1c/0x40
	   [<fa7b182c>] ? wmi_query_block+0x15a/0x162 [wmi]
	   [<fa7b182c>] ? wmi_query_block+0x15a/0x162 [wmi]
	   [<fa7e7000>] ? acer_wmi_init+0x00/0x61a [acer_wmi]
	   [<fa7e7135>] ? acer_wmi_init+0x135/0x61a [acer_wmi]
	   [<c0101159>] ? do_one_initcall+0x50+0x126
Quoted 3 lines
Quoted 1 line
Signed-off-by Costantino Leandro <[email protected]>
Acked-by Carlos Corbacho <[email protected]>
Cc [email protected]

Since I'm starting to get quite a few bug reports from people about 2.6.30 when distributions are turning on stack protector.

-Carlos

Signature · 3 lines
E-Mail: [email protected]
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D
#3 lore raw
Quoted 5 lines · On Friday 12 June 2009 06:44:18 Carlos Corbacho wrote:
Quoted 4 lines · On Tuesday 09 Jun 2009 20:46:24 Carlos Corbacho wrote:

[Adding Len to CC]

Quoted 1 line · On Tuesday 09 Jun 2009 22:09:23 [email protected] wrote:

From: Costantino Leandro <[email protected]>

[..]

Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Costantino Leandro <[email protected]>
Acked-by Carlos Corbacho <[email protected]>
Cc [email protected]

Since I'm starting to get quite a few bug reports from people about 2.6.30 when distributions are turning on stack protector.

Len,

Ping? I'm still not seeing this patch in the ACPI or Linus' tree...

-Carlos

Signature · 3 lines
E-Mail: [email protected]
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D