Skip to contentSkip to conversation

acpi ->video_device_list corruption

PATCH Applied William Lee Irwin III 3 participants 5 messages Dec 12, 2007 – Dec 13, 2007 ACPI

lore
Share & tools
lore
#0 lore raw

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

-- wli

drivers/acpi/video.c +2 -2
index 44a0d9b..7895d57 100644
@@ -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;
 	}
#1 lore raw
Quoted 21 lines · William Lee Irwin III

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

-- wli

drivers/acpi/video.c +0 -0
index 44a0d9b..7895d57 100644

drivers/acpi/video.c static void acpi_video_device_find_cap(struct acpi_video_device *device) · line 580

Quoted 5 lines
drivers/acpi/video.c +1 -1
@@ -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.

#2 lore raw
Quoted 2 lines · Mikael Pettersson

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

drivers/acpi/video.c +2 -2
index 44a0d9b..bd77e81 100644
@@ -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;
 	}
#3 lore raw
Quoted 5 lines · William Lee Irwin III
Quoted 2 lines · Mikael Pettersson

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.

Quoted 41 lines

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

drivers/acpi/video.c +2 -3
index 44a0d9b..bd77e81 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/