* acpi ->video_device_list corruption
@ 2007-12-12 10:15 William Lee Irwin III
2007-12-12 11:48 ` Mikael Pettersson
0 siblings, 1 reply; 5+ messages in thread
From: William Lee Irwin III @ 2007-12-12 10:15 UTC (permalink / raw)
To: lenb; +Cc: linux-acpi, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1365 bytes --]
The ->cap fields of struct acpi_video_device and struct acpi_video_bus
are 1B each, not 4B. The oversized memset()'s corrupted the subsequent
list_head fields. This resulted in silent corruption without
CONFIG_DEBUG_LIST and BUG's with it. This patch uses sizeof() to pass
the proper bounds to the memset() calls and thereby correct the bugs.
Included as a MIME attachment is a compressed dmesg from an affected
system. The patch was seen to resolve the issue on the affected system.
vs. 2.6.24-rc5
Signed-off-by: William Irwin <wli@holomorphy.com>
-- wli
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 44a0d9b..7895d57 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -577,7 +577,7 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
struct acpi_video_device_brightness *br = NULL;
- memset(&device->cap, 0, 4);
+ memset(&device->cap, 0, sizeof(struct acpi_video_device_cap));
if (ACPI_SUCCESS(acpi_get_handle(device->dev->handle, "_ADR", &h_dummy1))) {
device->cap._ADR = 1;
@@ -697,7 +697,7 @@ static void acpi_video_bus_find_cap(struct acpi_video_bus *video)
{
acpi_handle h_dummy1;
- memset(&video->cap, 0, 4);
+ memset(&video->cap, 0, sizeof(struct acpi_video_bus_cap));
if (ACPI_SUCCESS(acpi_get_handle(video->device->handle, "_DOS", &h_dummy1))) {
video->cap._DOS = 1;
}
[-- Attachment #2: dmesg.acpibug.gz --]
[-- Type: application/octet-stream, Size: 14584 bytes --]
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: acpi ->video_device_list corruption
2007-12-12 10:15 acpi ->video_device_list corruption William Lee Irwin III
@ 2007-12-12 11:48 ` Mikael Pettersson
2007-12-12 11:56 ` William Lee Irwin III
0 siblings, 1 reply; 5+ messages in thread
From: Mikael Pettersson @ 2007-12-12 11:48 UTC (permalink / raw)
To: William Lee Irwin III; +Cc: lenb, linux-acpi, linux-kernel
William Lee Irwin III writes:
> The ->cap fields of struct acpi_video_device and struct acpi_video_bus
> are 1B each, not 4B. The oversized memset()'s corrupted the subsequent
> list_head fields. This resulted in silent corruption without
> CONFIG_DEBUG_LIST and BUG's with it. This patch uses sizeof() to pass
> the proper bounds to the memset() calls and thereby correct the bugs.
>
> Included as a MIME attachment is a compressed dmesg from an affected
> system. The patch was seen to resolve the issue on the affected system.
>
> vs. 2.6.24-rc5
>
> Signed-off-by: William Irwin <wli@holomorphy.com>
>
>
> -- wli
>
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 44a0d9b..7895d57 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -577,7 +577,7 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
> struct acpi_video_device_brightness *br = NULL;
>
>
> - memset(&device->cap, 0, 4);
> + memset(&device->cap, 0, sizeof(struct acpi_video_device_cap));
IMO the memset(ptr, 0, sizeof(*ptr)) idiom is both safer
and avoids having to write an uninteresting type name.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: acpi ->video_device_list corruption
2007-12-12 11:48 ` Mikael Pettersson
@ 2007-12-12 11:56 ` William Lee Irwin III
2007-12-12 12:12 ` Mikael Pettersson
0 siblings, 1 reply; 5+ messages in thread
From: William Lee Irwin III @ 2007-12-12 11:56 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: lenb, linux-acpi, linux-kernel
On Wed, Dec 12, 2007 at 12:48:09PM +0100, Mikael Pettersson wrote:
> IMO the memset(ptr, 0, sizeof(*ptr)) idiom is both safer
> and avoids having to write an uninteresting type name.
How about this, then?
The ->cap fields of struct acpi_video_device and struct acpi_video_bus
are 1B each, not 4B. The oversized memset()'s corrupted the subsequent
list_head fields. This resulted in silent corruption without
CONFIG_DEBUG_LIST and BUG's with it. This patch uses sizeof() to pass
the proper bounds to the memset() calls and thereby correct the bugs.
The patch was seen to resolve the issue on the affected system.
vs. 2.6.24-rc5
Signed-off-by: William Irwin <wli@holomorphy.com>
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 44a0d9b..bd77e81 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -577,7 +577,7 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
struct acpi_video_device_brightness *br = NULL;
- memset(&device->cap, 0, 4);
+ memset(&device->cap, 0, sizeof(device->cap));
if (ACPI_SUCCESS(acpi_get_handle(device->dev->handle, "_ADR", &h_dummy1))) {
device->cap._ADR = 1;
@@ -697,7 +697,7 @@ static void acpi_video_bus_find_cap(struct acpi_video_bus *video)
{
acpi_handle h_dummy1;
- memset(&video->cap, 0, 4);
+ memset(&video->cap, 0, sizeof(video->cap));
if (ACPI_SUCCESS(acpi_get_handle(video->device->handle, "_DOS", &h_dummy1))) {
video->cap._DOS = 1;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: acpi ->video_device_list corruption
2007-12-12 11:56 ` William Lee Irwin III
@ 2007-12-12 12:12 ` Mikael Pettersson
2007-12-13 21:24 ` Len Brown
0 siblings, 1 reply; 5+ messages in thread
From: Mikael Pettersson @ 2007-12-12 12:12 UTC (permalink / raw)
To: William Lee Irwin III; +Cc: Mikael Pettersson, lenb, linux-acpi, linux-kernel
William Lee Irwin III writes:
> On Wed, Dec 12, 2007 at 12:48:09PM +0100, Mikael Pettersson wrote:
> > IMO the memset(ptr, 0, sizeof(*ptr)) idiom is both safer
> > and avoids having to write an uninteresting type name.
>
> How about this, then?
Looks good.
Acked-by: Mikael Pettersson <mikpe@it.uu.se>
>
> The ->cap fields of struct acpi_video_device and struct acpi_video_bus
> are 1B each, not 4B. The oversized memset()'s corrupted the subsequent
> list_head fields. This resulted in silent corruption without
> CONFIG_DEBUG_LIST and BUG's with it. This patch uses sizeof() to pass
> the proper bounds to the memset() calls and thereby correct the bugs.
>
> The patch was seen to resolve the issue on the affected system.
>
> vs. 2.6.24-rc5
>
> Signed-off-by: William Irwin <wli@holomorphy.com>
>
> diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
> index 44a0d9b..bd77e81 100644
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -577,7 +577,7 @@ static void acpi_video_device_find_cap(struct acpi_video_device *device)
> struct acpi_video_device_brightness *br = NULL;
>
>
> - memset(&device->cap, 0, 4);
> + memset(&device->cap, 0, sizeof(device->cap));
>
> if (ACPI_SUCCESS(acpi_get_handle(device->dev->handle, "_ADR", &h_dummy1))) {
> device->cap._ADR = 1;
> @@ -697,7 +697,7 @@ static void acpi_video_bus_find_cap(struct acpi_video_bus *video)
> {
> acpi_handle h_dummy1;
>
> - memset(&video->cap, 0, 4);
> + memset(&video->cap, 0, sizeof(video->cap));
> if (ACPI_SUCCESS(acpi_get_handle(video->device->handle, "_DOS", &h_dummy1))) {
> video->cap._DOS = 1;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-12-13 21:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-12-12 10:15 acpi ->video_device_list corruption William Lee Irwin III
2007-12-12 11:48 ` Mikael Pettersson
2007-12-12 11:56 ` William Lee Irwin III
2007-12-12 12:12 ` Mikael Pettersson
2007-12-13 21:24 ` Len Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox