linux-kernel - RE: [PATCH] ACPI: scan: Fix a Hyper-V Linux VM panic caused by buffer overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 9 Jan 2021 09:37:41 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
CC:     "rafael@...nel.org" <rafael@...nel.org>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "rjw@...ysocki.net" <rjw@...ysocki.net>,
        "len.brown@...el.com" <len.brown@...el.com>,
        Michael Kelley <mikelley@...rosoft.com>,
        "rui.zhang@...el.com" <rui.zhang@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        KY Srinivasan <kys@...rosoft.com>,
        "dwaipayanray1@...il.com" <dwaipayanray1@...il.com>
Subject: RE: [PATCH] ACPI: scan: Fix a Hyper-V Linux VM panic caused by buffer
 overflow

> From: Andy Shevchenko <andy.shevchenko@...il.com> 
> Sent: Saturday, January 9, 2021 12:52 AM
>> 
>> Hi Rafael, Len, and all,
>> Can you please take a look at the v2 patch?
>> 
>> The Linux mainline has been broken for several weeks when it
>> runs as a guest on Hyper-V, so we'd like this to be fixed ASAP,
>> as more people are being affected
> 
> I would like to see a warning printed when the dupped
> string violates the spec.

Hi Andy,
Do you want a simple strlen() check like the below, or a full
check of the AAA#### or NNNN#### format?

Can we have the v2 (https://lkml.org/lkml/2021/1/8/53) merged 
first, and then we can add another patch for the format checking?

I'm trying to do one thing in one patch so the patch is small enough
for easy reviewing.

diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index cb229e24c563..e6a5d997241c 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -97,7 +97,7 @@ void acpi_scan_table_handler(u32 event, void *table, void *context);
 extern struct list_head acpi_bus_id_list;
 
 struct acpi_device_bus_id {
-	char bus_id[15];
+	const char *bus_id;
 	unsigned int instance_no;
 	struct list_head node;
 };
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index a1b226eb2ce2..3b9902e5d965 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -486,6 +486,7 @@ static void acpi_device_del(struct acpi_device *device)
 				acpi_device_bus_id->instance_no--;
 			else {
 				list_del(&acpi_device_bus_id->node);
+				kfree_const(acpi_device_bus_id->bus_id);
 				kfree(acpi_device_bus_id);
 			}
 			break;
@@ -674,7 +675,23 @@ int acpi_device_add(struct acpi_device *device,
 	}
 	if (!found) {
 		acpi_device_bus_id = new_bus_id;
-		strcpy(acpi_device_bus_id->bus_id, acpi_device_hid(device));
+		acpi_device_bus_id->bus_id =
+			kstrdup_const(acpi_device_hid(device), GFP_KERNEL);
+		if (!acpi_device_bus_id->bus_id) {
+			pr_err(PREFIX "Memory allocation error for bus id\n");
+			result = -ENOMEM;
+			goto err_free_new_bus_id;
+		}
+
+		/*
+		 *  ACPI Spec v6.2, Section 6.1.5 _HID (Hardware ID): if the
+		 * ID is a string, it must be of the form AAA#### or NNNN####,
+		 * i.e. 7 chars or 8 characters.
+		 */
+		if (strlen(acpi_device_bus_id->bus_id) > 8)
+			pr_warn(PREFIX "too long HID name: %s\n",
+				acpi_device_bus_id->bus_id);
+
 		acpi_device_bus_id->instance_no = 0;
 		list_add_tail(&acpi_device_bus_id->node, &acpi_bus_id_list);
 	}
@@ -709,6 +726,10 @@ int acpi_device_add(struct acpi_device *device,
 	if (device->parent)
 		list_del(&device->node);
 	list_del(&device->wakeup_list);
+
+ err_free_new_bus_id:
+	if (!found)
+		kfree(new_bus_id);
 	mutex_unlock(&acpi_device_lock);
 
  err_detach: