From 689159fedc6cefecaaa8b72571cf04e0b1ef4232 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Sun, 18 May 2014 05:52:09 +0000 Subject: [PATCH] [ACPI] - Fix a buffer overrun that caused a BSOD with ACPI enabled on Hyper-V - Dynamically allocate the hardware ID buffer to prevent another HID overrun - Switched sprintf to snprintf to prevent this from happening to another ID svn path=/trunk/; revision=63344 --- reactos/drivers/bus/acpi/busmgr/bus.c | 45 ++++++++++++--------- reactos/drivers/bus/acpi/include/acpi_bus.h | 6 +-- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/reactos/drivers/bus/acpi/busmgr/bus.c b/reactos/drivers/bus/acpi/busmgr/bus.c index 77b822d2149..ebdb3a62c1b 100644 --- a/reactos/drivers/bus/acpi/busmgr/bus.c +++ b/reactos/drivers/bus/acpi/busmgr/bus.c @@ -1142,7 +1142,7 @@ acpi_bus_add ( char *uid = NULL; ACPI_PNP_DEVICE_ID_LIST *cid_list = NULL; int i = 0; - char static_uid_buffer[5]; + acpi_unique_id static_uid_buffer; if (!child) return_VALUE(AE_BAD_PARAMETER); @@ -1165,15 +1165,15 @@ acpi_bus_add ( */ switch (type) { case ACPI_BUS_TYPE_SYSTEM: - sprintf(device->pnp.bus_id, "%s", "ACPI"); + snprintf(device->pnp.bus_id, sizeof(device->pnp.bus_id), "%s", "ACPI"); break; case ACPI_BUS_TYPE_POWER_BUTTONF: case ACPI_BUS_TYPE_POWER_BUTTON: - sprintf(device->pnp.bus_id, "%s", "PWRF"); + snprintf(device->pnp.bus_id, sizeof(device->pnp.bus_id), "%s", "PWRF"); break; case ACPI_BUS_TYPE_SLEEP_BUTTONF: case ACPI_BUS_TYPE_SLEEP_BUTTON: - sprintf(device->pnp.bus_id, "%s", "SLPF"); + snprintf(device->pnp.bus_id, sizeof(device->pnp.bus_id), "%s", "SLPF"); break; default: buffer.Length = sizeof(bus_id); @@ -1188,7 +1188,7 @@ acpi_bus_add ( else break; } - sprintf(device->pnp.bus_id, "%s", bus_id); + snprintf(device->pnp.bus_id, sizeof(device->pnp.bus_id), "%s", bus_id); buffer.Pointer = NULL; /* HACK: Skip HPET */ @@ -1277,12 +1277,12 @@ acpi_bus_add ( case ACPI_BUS_TYPE_POWER: hid = ACPI_POWER_HID; uid = static_uid_buffer; - sprintf(uid, "%d", (PowerDeviceCount++)); + snprintf(uid, sizeof(static_uid_buffer), "%d", (PowerDeviceCount++)); break; case ACPI_BUS_TYPE_PROCESSOR: hid = ACPI_PROCESSOR_HID; uid = static_uid_buffer; - sprintf(uid, "_%d", (ProcessorCount++)); + snprintf(uid, sizeof(static_uid_buffer), "_%d", (ProcessorCount++)); break; case ACPI_BUS_TYPE_SYSTEM: hid = ACPI_SYSTEM_HID; @@ -1290,27 +1290,27 @@ acpi_bus_add ( case ACPI_BUS_TYPE_THERMAL: hid = ACPI_THERMAL_HID; uid = static_uid_buffer; - sprintf(uid, "%d", (ThermalZoneCount++)); + snprintf(uid, sizeof(static_uid_buffer), "%d", (ThermalZoneCount++)); break; case ACPI_BUS_TYPE_POWER_BUTTON: hid = ACPI_BUTTON_HID_POWER; uid = static_uid_buffer; - sprintf(uid, "%d", (PowerButtonCount++)); + snprintf(uid, sizeof(static_uid_buffer), "%d", (PowerButtonCount++)); break; case ACPI_BUS_TYPE_POWER_BUTTONF: hid = ACPI_BUTTON_HID_POWERF; uid = static_uid_buffer; - sprintf(uid, "%d", (FixedPowerButtonCount++)); + snprintf(uid, sizeof(static_uid_buffer), "%d", (FixedPowerButtonCount++)); break; case ACPI_BUS_TYPE_SLEEP_BUTTON: hid = ACPI_BUTTON_HID_SLEEP; uid = static_uid_buffer; - sprintf(uid, "%d", (SleepButtonCount++)); + snprintf(uid, sizeof(static_uid_buffer), "%d", (SleepButtonCount++)); break; case ACPI_BUS_TYPE_SLEEP_BUTTONF: hid = ACPI_BUTTON_HID_SLEEPF; uid = static_uid_buffer; - sprintf(uid, "%d", (FixedSleepButtonCount++)); + snprintf(uid, sizeof(static_uid_buffer), "%d", (FixedSleepButtonCount++)); break; } @@ -1321,16 +1321,19 @@ acpi_bus_add ( */ if (((ACPI_HANDLE)parent == ACPI_ROOT_OBJECT) && (type == ACPI_BUS_TYPE_DEVICE)) { hid = ACPI_BUS_HID; - sprintf(device->pnp.device_name, "%s", ACPI_BUS_DEVICE_NAME); - sprintf(device->pnp.device_class, "%s", ACPI_BUS_CLASS); + snprintf(device->pnp.device_name, sizeof(device->pnp.device_name), "%s", ACPI_BUS_DEVICE_NAME); + snprintf(device->pnp.device_class, sizeof(device->pnp.device_class), "%s", ACPI_BUS_CLASS); } if (hid) { - sprintf(device->pnp.hardware_id, "%s", hid); - device->flags.hardware_id = 1; + device->pnp.hardware_id = ExAllocatePoolWithTag(NonPagedPool, strlen(hid) + 1, 'IPCA'); + if (device->pnp.hardware_id) { + snprintf(device->pnp.hardware_id, strlen(hid) + 1, "%s", hid); + device->flags.hardware_id = 1; + } } if (uid) { - sprintf(device->pnp.unique_id, "%s", uid); + snprintf(device->pnp.unique_id, sizeof(device->pnp.unique_id), "%s", uid); device->flags.unique_id = 1; } @@ -1434,6 +1437,9 @@ end: if (device->pnp.cid_list) { ExFreePoolWithTag(device->pnp.cid_list, 'IPCA'); } + if (device->pnp.hardware_id) { + ExFreePoolWithTag(device->pnp.hardware_id, 'IPCA'); + } ExFreePoolWithTag(device, 'IPCA'); return_VALUE(result); } @@ -1454,9 +1460,12 @@ acpi_bus_remove ( acpi_device_unregister(device); - if (device && device->pnp.cid_list) + if (device->pnp.cid_list) ExFreePoolWithTag(device->pnp.cid_list, 'IPCA'); + if (device->pnp.hardware_id) + ExFreePoolWithTag(device->pnp.hardware_id, 'IPCA'); + if (device) ExFreePoolWithTag(device, 'IPCA'); diff --git a/reactos/drivers/bus/acpi/include/acpi_bus.h b/reactos/drivers/bus/acpi/include/acpi_bus.h index 4320f63a1dd..148373f6608 100644 --- a/reactos/drivers/bus/acpi/include/acpi_bus.h +++ b/reactos/drivers/bus/acpi/include/acpi_bus.h @@ -164,10 +164,10 @@ struct acpi_device_flags { /* Plug and Play */ -typedef char acpi_bus_id[20]; +typedef char acpi_bus_id[8]; typedef unsigned long acpi_bus_address; -typedef char acpi_hardware_id[20]; -typedef char acpi_unique_id[20]; +typedef char *acpi_hardware_id; +typedef char acpi_unique_id[9]; typedef char acpi_device_name[40]; typedef char acpi_device_class[20];