From patchwork Fri Mar 1 07:51:21 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ivan Hu X-Patchwork-Id: 1906564 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=185.125.189.65; helo=lists.ubuntu.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=patchwork.ozlabs.org) Received: from lists.ubuntu.com (lists.ubuntu.com [185.125.189.65]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4TmKzz3TQRz23hc for ; Fri, 1 Mar 2024 18:51:59 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=lists.ubuntu.com) by lists.ubuntu.com with esmtp (Exim 4.86_2) (envelope-from ) id 1rfxgM-0004Ig-KU; Fri, 01 Mar 2024 07:51:50 +0000 Received: from smtp-relay-canonical-0.internal ([10.131.114.83] helo=smtp-relay-canonical-0.canonical.com) by lists.ubuntu.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1rfxgA-0004GW-Ko for kernel-team@lists.ubuntu.com; Fri, 01 Mar 2024 07:51:40 +0000 Received: from canonical.com (unknown [106.104.136.95]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-canonical-0.canonical.com (Postfix) with ESMTPSA id 47D923F2B2 for ; Fri, 1 Mar 2024 07:51:33 +0000 (UTC) From: Ivan Hu To: kernel-team@lists.ubuntu.com Subject: [SRU][M][PATCH v5 1/2] ACPI: utils: Dynamically determine acpi_handle_list size Date: Fri, 1 Mar 2024 15:51:21 +0800 Message-Id: <20240301075122.7242-2-ivan.hu@canonical.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240301075122.7242-1-ivan.hu@canonical.com> References: <20240301075122.7242-1-ivan.hu@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: "Rafael J. Wysocki" BugLink: https://bugs.launchpad.net/bugs/2049733 Address a long-standing "TBD" comment in the ACPI headers regarding the number of handles in struct acpi_handle_list. The number 10, which along with the comment dates back to 2.4.23, seems like it may have been arbitrarily chosen and isn't sufficient in all cases [1]. Finally change the code to dynamically determine the size of the handles table in struct acpi_handle_list and allocate it accordingly. Update the users of to struct acpi_handle_list to take the additional dynamic allocation into account. Link: https://lore.kernel.org/linux-acpi/20230809094451.15473-1-ivan.hu@canonical.com # [1] Co-developed-by: Vicki Pfau Signed-off-by: Vicki Pfau Signed-off-by: Rafael J. Wysocki (backported from commit 2e57d10a6591560724b80a628235559571f4cb8d) [Ivan Hu: ignore irrelevant moving PSL patches and rearrange the memset and compare] Signed-off-by: Ivan Hu --- drivers/acpi/acpi_lpss.c | 10 ++- drivers/acpi/scan.c | 1 + drivers/acpi/thermal.c | 29 ++++++--- drivers/acpi/utils.c | 61 ++++++++++++++++++- .../platform/surface/surface_acpi_notify.c | 10 ++- include/acpi/acpi_bus.h | 9 ++- 6 files changed, 100 insertions(+), 20 deletions(-) diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c index 539e700de4d2..3e756b9b5aa6 100644 --- a/drivers/acpi/acpi_lpss.c +++ b/drivers/acpi/acpi_lpss.c @@ -578,6 +578,7 @@ static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) { struct acpi_handle_list dep_devices; acpi_status status; + bool ret = false; int i; if (!acpi_has_method(adev->handle, "_DEP")) @@ -591,11 +592,14 @@ static bool acpi_lpss_dep(struct acpi_device *adev, acpi_handle handle) } for (i = 0; i < dep_devices.count; i++) { - if (dep_devices.handles[i] == handle) - return true; + if (dep_devices.handles[i] == handle) { + ret = true; + break; + } } - return false; + acpi_handle_list_free(&dep_devices); + return ret; } static void acpi_lpss_link_consumer(struct device *dev1, diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index c1cb6fcd7930..71b302c16a87 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2034,6 +2034,7 @@ static u32 acpi_scan_check_dep(acpi_handle handle, bool check_dep) mutex_unlock(&acpi_dep_list_lock); } + acpi_handle_list_free(&dep_devices); return count; } diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c index 3163a40f02e3..91aeea3ad766 100644 --- a/drivers/acpi/thermal.c +++ b/drivers/acpi/thermal.c @@ -190,7 +190,7 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag) { acpi_status status; unsigned long long tmp; - struct acpi_handle_list devices; + struct acpi_handle_list devices = { 0 }; bool valid = false; int i; @@ -294,7 +294,7 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag) } } if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.passive.valid) { - memset(&devices, 0, sizeof(struct acpi_handle_list)); + status = acpi_evaluate_reference(tz->device->handle, "_PSL", NULL, &devices); if (ACPI_FAILURE(status)) { @@ -305,12 +305,12 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag) tz->trips.passive.valid = true; } - if (memcmp(&tz->trips.passive.devices, &devices, - sizeof(struct acpi_handle_list))) { - memcpy(&tz->trips.passive.devices, &devices, - sizeof(struct acpi_handle_list)); - ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device"); + if (acpi_handle_list_equal(&tz->trips.passive.devices, &devices)) { + acpi_handle_list_free(&devices); + return 0; } + ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device"); + acpi_handle_list_replace(&tz->trips.passive.devices, &devices); } if ((flag & ACPI_TRIPS_PASSIVE) || (flag & ACPI_TRIPS_DEVICES)) { if (valid != tz->trips.passive.valid) @@ -959,6 +959,17 @@ static void acpi_thermal_check_fn(struct work_struct *work) mutex_unlock(&tz->thermal_check_lock); } +static void acpi_thermal_free_thermal_zone(struct acpi_thermal *tz) +{ + int i; + + acpi_handle_list_free(&tz->trips.passive.devices); + for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) + acpi_handle_list_free(&tz->trips.active[i].devices); + + kfree(tz); +} + static int acpi_thermal_add(struct acpi_device *device) { struct acpi_thermal *tz; @@ -996,7 +1007,7 @@ static int acpi_thermal_add(struct acpi_device *device) goto end; free_memory: - kfree(tz); + acpi_thermal_free_thermal_zone(tz); end: return result; } @@ -1012,7 +1023,7 @@ static void acpi_thermal_remove(struct acpi_device *device) tz = acpi_driver_data(device); acpi_thermal_unregister_thermal_zone(tz); - kfree(tz); + acpi_thermal_free_thermal_zone(tz); } #ifdef CONFIG_PM_SLEEP diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index 2ea14648a661..c6f83c21bb2a 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -370,7 +370,8 @@ acpi_evaluate_reference(acpi_handle handle, goto end; } - if (package->package.count > ACPI_MAX_HANDLES) { + list->handles = kcalloc(package->package.count, sizeof(*list->handles), GFP_KERNEL); + if (!list->handles) { kfree(package); return AE_NO_MEMORY; } @@ -402,7 +403,8 @@ acpi_evaluate_reference(acpi_handle handle, end: if (ACPI_FAILURE(status)) { list->count = 0; - //kfree(list->handles); + kfree(list->handles); + list->handles = NULL; } kfree(buffer.pointer); @@ -412,6 +414,61 @@ acpi_evaluate_reference(acpi_handle handle, EXPORT_SYMBOL(acpi_evaluate_reference); +/** + * acpi_handle_list_equal - Check if two ACPI handle lists are the same + * @list1: First list to compare. + * @list2: Second list to compare. + * + * Return true if the given ACPI handle lists are of the same size and + * contain the same ACPI handles in the same order. Otherwise, return false. + */ +bool acpi_handle_list_equal(struct acpi_handle_list *list1, + struct acpi_handle_list *list2) +{ + return list1->count == list2->count && + !memcmp(list1->handles, list2->handles, + list1->count * sizeof(acpi_handle)); +} +EXPORT_SYMBOL_GPL(acpi_handle_list_equal); + +/** + * acpi_handle_list_replace - Replace one ACPI handle list with another + * @dst: ACPI handle list to replace. + * @src: Source ACPI handle list. + * + * Free the handles table in @dst, move the handles table from @src to @dst, + * copy count from @src to @dst and clear @src. + */ +void acpi_handle_list_replace(struct acpi_handle_list *dst, + struct acpi_handle_list *src) +{ + if (dst->count) + kfree(dst->handles); + + dst->count = src->count; + dst->handles = src->handles; + + src->handles = NULL; + src->count = 0; +} +EXPORT_SYMBOL_GPL(acpi_handle_list_replace); + +/** + * acpi_handle_list_free - Free the handles table in an ACPI handle list + * @list: ACPI handle list to free. + * + * Free the handles table in @list and clear its count field. + */ +void acpi_handle_list_free(struct acpi_handle_list *list) +{ + if (!list->count) + return; + + kfree(list->handles); + list->count = 0; +} +EXPORT_SYMBOL_GPL(acpi_handle_list_free); + acpi_status acpi_get_physical_device_location(acpi_handle handle, struct acpi_pld_info **pld) { diff --git a/drivers/platform/surface/surface_acpi_notify.c b/drivers/platform/surface/surface_acpi_notify.c index 897cdd9c3aae..0412a644fece 100644 --- a/drivers/platform/surface/surface_acpi_notify.c +++ b/drivers/platform/surface/surface_acpi_notify.c @@ -741,6 +741,7 @@ static bool is_san_consumer(struct platform_device *pdev, acpi_handle handle) struct acpi_handle_list dep_devices; acpi_handle supplier = ACPI_HANDLE(&pdev->dev); acpi_status status; + bool ret = false; int i; if (!acpi_has_method(handle, "_DEP")) @@ -753,11 +754,14 @@ static bool is_san_consumer(struct platform_device *pdev, acpi_handle handle) } for (i = 0; i < dep_devices.count; i++) { - if (dep_devices.handles[i] == supplier) - return true; + if (dep_devices.handles[i] == supplier) { + ret = true; + break; + } } - return false; + acpi_handle_list_free(&dep_devices); + return ret; } static acpi_status san_consumer_setup(acpi_handle handle, u32 lvl, diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index db7514033a96..d5de93e1ac7c 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -12,11 +12,9 @@ #include #include -/* TBD: Make dynamic */ -#define ACPI_MAX_HANDLES 10 struct acpi_handle_list { u32 count; - acpi_handle handles[ACPI_MAX_HANDLES]; + acpi_handle* handles; }; /* acpi_utils.h */ @@ -32,6 +30,11 @@ acpi_evaluate_reference(acpi_handle handle, acpi_string pathname, struct acpi_object_list *arguments, struct acpi_handle_list *list); +bool acpi_handle_list_equal(struct acpi_handle_list *list1, + struct acpi_handle_list *list2); +void acpi_handle_list_replace(struct acpi_handle_list *dst, + struct acpi_handle_list *src); +void acpi_handle_list_free(struct acpi_handle_list *list); acpi_status acpi_evaluate_ost(acpi_handle handle, u32 source_event, u32 status_code, struct acpi_buffer *status_buf); From patchwork Fri Mar 1 07:51:22 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ivan Hu X-Patchwork-Id: 1906565 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=185.125.189.65; helo=lists.ubuntu.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=patchwork.ozlabs.org) Received: from lists.ubuntu.com (lists.ubuntu.com [185.125.189.65]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4TmL0073t6z23hc for ; Fri, 1 Mar 2024 18:52:00 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=lists.ubuntu.com) by lists.ubuntu.com with esmtp (Exim 4.86_2) (envelope-from ) id 1rfxgO-0004Jm-6P; Fri, 01 Mar 2024 07:51:52 +0000 Received: from smtp-relay-canonical-0.internal ([10.131.114.83] helo=smtp-relay-canonical-0.canonical.com) by lists.ubuntu.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1rfxgF-0004Hh-Tt for kernel-team@lists.ubuntu.com; Fri, 01 Mar 2024 07:51:44 +0000 Received: from canonical.com (unknown [106.104.136.95]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-canonical-0.canonical.com (Postfix) with ESMTPSA id 7845E3F2B2 for ; Fri, 1 Mar 2024 07:51:42 +0000 (UTC) From: Ivan Hu To: kernel-team@lists.ubuntu.com Subject: [SRU][M][PATCH v5 2/2] ACPI: utils: Fix error path in acpi_evaluate_reference() Date: Fri, 1 Mar 2024 15:51:22 +0800 Message-Id: <20240301075122.7242-3-ivan.hu@canonical.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20240301075122.7242-1-ivan.hu@canonical.com> References: <20240301075122.7242-1-ivan.hu@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: "Rafael J. Wysocki" BugLink: https://bugs.launchpad.net/bugs/2049733 If a pointer to an uninitialized struct acpi_handle_list is passed to acpi_evaluate_reference() and it decides to bail out early, either because acpi_evaluate_object() fails, or because it produces invalid data, the handles pointer from the struct acpi_handle_list will be passed to kfree() and if it is not NULL, the kernel will crash on an attempt to free unallocated memory. Address this by moving the "end" label in acpi_evaluate_reference() to the end of the function, which is sufficient, because no cleanup is needed in that case. Fixes: 2e57d10a6591 ("ACPI: utils: Dynamically determine acpi_handle_list size") Signed-off-by: Rafael J. Wysocki Tested-by: Woody Suwalski (backported from commit 8f0b960a42badda7a2781e8a33564624200debc9) [Ivan Hu: change spaces] Signed-off-by: Ivan Hu --- drivers/acpi/utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c index c6f83c21bb2a..7c8af9c14981 100644 --- a/drivers/acpi/utils.c +++ b/drivers/acpi/utils.c @@ -400,13 +400,13 @@ acpi_evaluate_reference(acpi_handle handle, acpi_handle_debug(list->handles[i], "Found in reference list\n"); } - end: if (ACPI_FAILURE(status)) { list->count = 0; kfree(list->handles); list->handles = NULL; } +end: kfree(buffer.pointer); return status;