From 83f5ad58daf1576640bec3aa2f81f003f8e6c48b Mon Sep 17 00:00:00 2001 From: Su Hui Date: Thu, 31 Aug 2023 15:34:56 +0800 Subject: [PATCH 1/8] ACPI: OSL: add __printf format attribute to acpi_os_vprintf() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With gcc and W=1 option to compile the kernel, warning occurs: drivers/acpi/osl.c:156:2: error: function ‘acpi_os_vprintf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]. Allow the compiler to recognize and check format strings is safer. Signed-off-by: Su Hui [ rjw: Subject and changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/osl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index f725813d0cce..357f1325485d 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -149,7 +149,7 @@ void acpi_os_printf(const char *fmt, ...) } EXPORT_SYMBOL(acpi_os_printf); -void acpi_os_vprintf(const char *fmt, va_list args) +void __printf(1, 0) acpi_os_vprintf(const char *fmt, va_list args) { static char buffer[512]; From f1fce1cf4509aa0676b363c203b9ab190c1fd8e8 Mon Sep 17 00:00:00 2001 From: Justin Stitt Date: Mon, 11 Sep 2023 20:36:44 +0000 Subject: [PATCH 2/8] ACPI: OSI: refactor deprecated strncpy() `strncpy()` is deprecated for use on NUL-terminated destination strings [1]. We know `osi->string` is a NUL-terminated string due to its eventual use in `acpi_install_interface()` and `acpi_remove_interface()` which expect a `acpi_string` which has been specifically typedef'd as: | typedef char *acpi_string; /* Null terminated ASCII string */ ... and which also has other string functions used on it like `strlen`. Furthermore, padding is not needed in this instance either. Due to the reasoning above a suitable replacement is `strscpy` [2] since it guarantees NUL-termination on the destination buffer and doesn't unnecessarily NUL-pad. While there is unlikely to be a buffer overread (or other related bug) in this case, we should still favor a more robust and less ambiguous interface. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Signed-off-by: Justin Stitt Reviewed-by: Kees Cook Signed-off-by: Rafael J. Wysocki --- drivers/acpi/osi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c index d4405e1ca9b9..df9328c850bd 100644 --- a/drivers/acpi/osi.c +++ b/drivers/acpi/osi.c @@ -110,7 +110,7 @@ void __init acpi_osi_setup(char *str) break; } else if (osi->string[0] == '\0') { osi->enable = enable; - strncpy(osi->string, str, OSI_STRING_LENGTH_MAX); + strscpy(osi->string, str, OSI_STRING_LENGTH_MAX); break; } } From 92002fb39e8e4a199231c1c842b07dbbd806ba75 Mon Sep 17 00:00:00 2001 From: Jonathan Bergh Date: Sat, 30 Sep 2023 11:02:48 +0200 Subject: [PATCH 3/8] ACPI: OSL: Fix up white space in parameter lists Fix up declarations of pointer arguments where a space is present before the argument name, which does not agree with the kernel coding style. Signed-off-by: Jonathan Bergh [ rjw: Subject and changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/osl.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 357f1325485d..1ccc9a8d258d 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -493,7 +493,7 @@ EXPORT_SYMBOL(acpi_os_unmap_generic_address); #ifdef ACPI_FUTURE_USAGE acpi_status -acpi_os_get_physical_address(void *virt, acpi_physical_address * phys) +acpi_os_get_physical_address(void *virt, acpi_physical_address *phys) { if (!phys || !virt) return AE_BAD_PARAMETER; @@ -784,7 +784,7 @@ acpi_os_write_memory(acpi_physical_address phys_addr, u64 value, u32 width) #ifdef CONFIG_PCI acpi_status -acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg, +acpi_os_read_pci_configuration(struct acpi_pci_id *pci_id, u32 reg, u64 *value, u32 width) { int result, size; @@ -816,7 +816,7 @@ acpi_os_read_pci_configuration(struct acpi_pci_id * pci_id, u32 reg, } acpi_status -acpi_os_write_pci_configuration(struct acpi_pci_id * pci_id, u32 reg, +acpi_os_write_pci_configuration(struct acpi_pci_id *pci_id, u32 reg, u64 value, u32 width) { int result, size; @@ -1197,7 +1197,7 @@ bool acpi_queue_hotplug_work(struct work_struct *work) } acpi_status -acpi_os_create_semaphore(u32 max_units, u32 initial_units, acpi_handle * handle) +acpi_os_create_semaphore(u32 max_units, u32 initial_units, acpi_handle *handle) { struct semaphore *sem = NULL; @@ -1554,7 +1554,7 @@ void acpi_os_release_lock(acpi_spinlock lockp, acpi_cpu_flags flags) ******************************************************************************/ acpi_status -acpi_os_create_cache(char *name, u16 size, u16 depth, acpi_cache_t ** cache) +acpi_os_create_cache(char *name, u16 size, u16 depth, acpi_cache_t **cache) { *cache = kmem_cache_create(name, size, 0, 0, NULL); if (*cache == NULL) @@ -1575,7 +1575,7 @@ acpi_os_create_cache(char *name, u16 size, u16 depth, acpi_cache_t ** cache) * ******************************************************************************/ -acpi_status acpi_os_purge_cache(acpi_cache_t * cache) +acpi_status acpi_os_purge_cache(acpi_cache_t *cache) { kmem_cache_shrink(cache); return (AE_OK); @@ -1594,7 +1594,7 @@ acpi_status acpi_os_purge_cache(acpi_cache_t * cache) * ******************************************************************************/ -acpi_status acpi_os_delete_cache(acpi_cache_t * cache) +acpi_status acpi_os_delete_cache(acpi_cache_t *cache) { kmem_cache_destroy(cache); return (AE_OK); @@ -1614,7 +1614,7 @@ acpi_status acpi_os_delete_cache(acpi_cache_t * cache) * ******************************************************************************/ -acpi_status acpi_os_release_object(acpi_cache_t * cache, void *object) +acpi_status acpi_os_release_object(acpi_cache_t *cache, void *object) { kmem_cache_free(cache, object); return (AE_OK); From 9d4e27dd88275a8e2a14521b63792f33edca5758 Mon Sep 17 00:00:00 2001 From: Jonathan Bergh Date: Sat, 30 Sep 2023 11:04:21 +0200 Subject: [PATCH 4/8] ACPI: OSL: Remove redundant parentheses in return statements Fix up three return statements including redundant perens around the return value. Signed-off-by: Jonathan Bergh [ rjw: Subject and changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/osl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 1ccc9a8d258d..838677010ceb 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -1578,7 +1578,7 @@ acpi_os_create_cache(char *name, u16 size, u16 depth, acpi_cache_t **cache) acpi_status acpi_os_purge_cache(acpi_cache_t *cache) { kmem_cache_shrink(cache); - return (AE_OK); + return AE_OK; } /******************************************************************************* @@ -1597,7 +1597,7 @@ acpi_status acpi_os_purge_cache(acpi_cache_t *cache) acpi_status acpi_os_delete_cache(acpi_cache_t *cache) { kmem_cache_destroy(cache); - return (AE_OK); + return AE_OK; } /******************************************************************************* @@ -1617,7 +1617,7 @@ acpi_status acpi_os_delete_cache(acpi_cache_t *cache) acpi_status acpi_os_release_object(acpi_cache_t *cache, void *object) { kmem_cache_free(cache, object); - return (AE_OK); + return AE_OK; } #endif From a1da3b78c08357f67adecf3b2e2be7d13aaccca7 Mon Sep 17 00:00:00 2001 From: Jonathan Bergh Date: Sat, 30 Sep 2023 11:04:36 +0200 Subject: [PATCH 5/8] ACPI: OSL: Add empty lines after local variable declarations Fix up four places where there is no empty line after declarations of local variables in a function (as per the kernel coding style). Signed-off-by: Jonathan Bergh [ rjw: Subject and changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/osl.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 838677010ceb..e154ad0be263 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -1067,6 +1067,7 @@ acpi_status acpi_os_execute(acpi_execute_type type, struct acpi_os_dpc *dpc; struct workqueue_struct *queue; int ret; + ACPI_DEBUG_PRINT((ACPI_DB_EXEC, "Scheduling function [%p(%p)] for deferred execution.\n", function, context)); @@ -1522,6 +1523,7 @@ acpi_cpu_flags acpi_os_acquire_lock(acpi_spinlock lockp) __acquires(lockp) { acpi_cpu_flags flags; + spin_lock_irqsave(lockp, flags); return flags; } @@ -1708,6 +1710,7 @@ acpi_status acpi_os_prepare_sleep(u8 sleep_state, u32 pm1a_control, u32 pm1b_control) { int rc = 0; + if (__acpi_os_prepare_sleep) rc = __acpi_os_prepare_sleep(sleep_state, pm1a_control, pm1b_control); @@ -1730,6 +1733,7 @@ acpi_status acpi_os_prepare_extended_sleep(u8 sleep_state, u32 val_a, u32 val_b) { int rc = 0; + if (__acpi_os_prepare_extended_sleep) rc = __acpi_os_prepare_extended_sleep(sleep_state, val_a, val_b); From a83c68a3bf7c418c9a46693c63c638852b0c1f4e Mon Sep 17 00:00:00 2001 From: Vasily Khoruzhick Date: Wed, 27 Sep 2023 12:50:02 -0700 Subject: [PATCH 6/8] ACPI: FPDT: properly handle invalid FPDT subtables Buggy BIOSes may have invalid FPDT subtables, e.g. on my hardware: S3PT subtable: 7F20FE30: 53 33 50 54 24 00 00 00-00 00 00 00 00 00 18 01 *S3PT$...........* 7F20FE40: 00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 *................* 7F20FE50: 00 00 00 00 Here the first record has zero length. FBPT subtable: 7F20FE50: 46 42 50 54-3C 00 00 00 46 42 50 54 *....FBPT<...FBPT* 7F20FE60: 02 00 30 02 00 00 00 00-00 00 00 00 00 00 00 00 *..0.............* 7F20FE70: 2A A6 BC 6E 0B 00 00 00-1A 44 41 70 0B 00 00 00 **..n.....DAp....* 7F20FE80: 00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 *................* And here FBPT table has FBPT signature repeated instead of the first record. Current code will be looping indefinitely due to zero length records, so break out of the loop if record length is zero. While we are here, add proper handling for fpdt_process_subtable() failures. Fixes: d1eb86e59be0 ("ACPI: tables: introduce support for FPDT table") Cc: All applicable Signed-off-by: Vasily Khoruzhick [ rjw: Comment edit, added empty code lines ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/acpi_fpdt.c | 45 +++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/drivers/acpi/acpi_fpdt.c b/drivers/acpi/acpi_fpdt.c index a2056c4c8cb7..271092f2700a 100644 --- a/drivers/acpi/acpi_fpdt.c +++ b/drivers/acpi/acpi_fpdt.c @@ -194,12 +194,19 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type) record_header = (void *)subtable_header + offset; offset += record_header->length; + if (!record_header->length) { + pr_err(FW_BUG "Zero-length record found in FPTD.\n"); + result = -EINVAL; + goto err; + } + switch (record_header->type) { case RECORD_S3_RESUME: if (subtable_type != SUBTABLE_S3PT) { pr_err(FW_BUG "Invalid record %d for subtable %s\n", record_header->type, signature); - return -EINVAL; + result = -EINVAL; + goto err; } if (record_resume) { pr_err("Duplicate resume performance record found.\n"); @@ -208,7 +215,7 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type) record_resume = (struct resume_performance_record *)record_header; result = sysfs_create_group(fpdt_kobj, &resume_attr_group); if (result) - return result; + goto err; break; case RECORD_S3_SUSPEND: if (subtable_type != SUBTABLE_S3PT) { @@ -223,13 +230,14 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type) record_suspend = (struct suspend_performance_record *)record_header; result = sysfs_create_group(fpdt_kobj, &suspend_attr_group); if (result) - return result; + goto err; break; case RECORD_BOOT: if (subtable_type != SUBTABLE_FBPT) { pr_err(FW_BUG "Invalid %d for subtable %s\n", record_header->type, signature); - return -EINVAL; + result = -EINVAL; + goto err; } if (record_boot) { pr_err("Duplicate boot performance record found.\n"); @@ -238,7 +246,7 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type) record_boot = (struct boot_performance_record *)record_header; result = sysfs_create_group(fpdt_kobj, &boot_attr_group); if (result) - return result; + goto err; break; default: @@ -247,6 +255,18 @@ static int fpdt_process_subtable(u64 address, u32 subtable_type) } } return 0; + +err: + if (record_boot) + sysfs_remove_group(fpdt_kobj, &boot_attr_group); + + if (record_suspend) + sysfs_remove_group(fpdt_kobj, &suspend_attr_group); + + if (record_resume) + sysfs_remove_group(fpdt_kobj, &resume_attr_group); + + return result; } static int __init acpi_init_fpdt(void) @@ -255,6 +275,7 @@ static int __init acpi_init_fpdt(void) struct acpi_table_header *header; struct fpdt_subtable_entry *subtable; u32 offset = sizeof(*header); + int result; status = acpi_get_table(ACPI_SIG_FPDT, 0, &header); @@ -263,8 +284,8 @@ static int __init acpi_init_fpdt(void) fpdt_kobj = kobject_create_and_add("fpdt", acpi_kobj); if (!fpdt_kobj) { - acpi_put_table(header); - return -ENOMEM; + result = -ENOMEM; + goto err_nomem; } while (offset < header->length) { @@ -272,8 +293,10 @@ static int __init acpi_init_fpdt(void) switch (subtable->type) { case SUBTABLE_FBPT: case SUBTABLE_S3PT: - fpdt_process_subtable(subtable->address, + result = fpdt_process_subtable(subtable->address, subtable->type); + if (result) + goto err_subtable; break; default: /* Other types are reserved in ACPI 6.4 spec. */ @@ -282,6 +305,12 @@ static int __init acpi_init_fpdt(void) offset += sizeof(*subtable); } return 0; +err_subtable: + kobject_put(fpdt_kobj); + +err_nomem: + acpi_put_table(header); + return result; } fs_initcall(acpi_init_fpdt); From b5bdb60faaaff11bcfc9a90372158bc56e7ff544 Mon Sep 17 00:00:00 2001 From: James Morse Date: Fri, 20 Oct 2023 14:59:48 +0100 Subject: [PATCH 7/8] ACPI: scan: Use the acpi_device_is_present() helper in more places acpi_device_is_present() checks the present or functional bits from the cached copy of _STA. A few places open-code this check. Use the helper instead to improve readability. Signed-off-by: James Morse Reviewed-by: Jonathan Cameron Reviewed-by: Gavin Shan Signed-off-by: Russell King (Oracle) Reviewed-by: Miguel Luis Signed-off-by: Rafael J. Wysocki --- drivers/acpi/scan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 691d4b7686ee..ed01e19514ef 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -304,7 +304,7 @@ static int acpi_scan_device_check(struct acpi_device *adev) int error; acpi_bus_get_status(adev); - if (adev->status.present || adev->status.functional) { + if (acpi_device_is_present(adev)) { /* * This function is only called for device objects for which * matching scan handlers exist. The only situation in which @@ -338,7 +338,7 @@ static int acpi_scan_bus_check(struct acpi_device *adev, void *not_used) int error; acpi_bus_get_status(adev); - if (!(adev->status.present || adev->status.functional)) { + if (!acpi_device_is_present(adev)) { acpi_scan_device_not_present(adev); return 0; } From 8c6fdbd635d4afbcd57243083319a55624d2e168 Mon Sep 17 00:00:00 2001 From: James Morse Date: Fri, 20 Oct 2023 19:47:04 +0100 Subject: [PATCH 8/8] ACPI: scan: Rename acpi_scan_device_not_present() to be about enumeration acpi_scan_device_not_present() is called when a device in the hierarchy is not available for enumeration. Historically enumeration was only based on whether the device was present. To add support for only enumerating devices that are both present and enabled, this helper should be renamed. It was only ever about enumeration, rename it acpi_scan_device_not_enumerated(). No change in behaviour is intended. Signed-off-by: James Morse Reviewed-by: Gavin Shan Signed-off-by: Russell King (Oracle) Reviewed-by: Miguel Luis Signed-off-by: Rafael J. Wysocki --- drivers/acpi/scan.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index ed01e19514ef..17ab875a7d4e 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -289,10 +289,10 @@ static int acpi_scan_hot_remove(struct acpi_device *device) return 0; } -static int acpi_scan_device_not_present(struct acpi_device *adev) +static int acpi_scan_device_not_enumerated(struct acpi_device *adev) { if (!acpi_device_enumerated(adev)) { - dev_warn(&adev->dev, "Still not present\n"); + dev_warn(&adev->dev, "Still not enumerated\n"); return -EALREADY; } acpi_bus_trim(adev); @@ -327,7 +327,7 @@ static int acpi_scan_device_check(struct acpi_device *adev) error = -ENODEV; } } else { - error = acpi_scan_device_not_present(adev); + error = acpi_scan_device_not_enumerated(adev); } return error; } @@ -339,7 +339,7 @@ static int acpi_scan_bus_check(struct acpi_device *adev, void *not_used) acpi_bus_get_status(adev); if (!acpi_device_is_present(adev)) { - acpi_scan_device_not_present(adev); + acpi_scan_device_not_enumerated(adev); return 0; } if (handler && handler->hotplug.scan_dependent)