Module Name: src Committed By: martin Date: Fri Aug 16 15:24:41 UTC 2019
Modified Files: src/sys/dev/acpi [netbsd-8]: acpi_ec.c Log Message: Pull up following revision(s) (requested by msaitoh in ticket #1337): sys/dev/acpi/acpi_ec.c: revision 1.76 sys/dev/acpi/acpi_ec.c: revision 1.77 - Fix a bug that acpiec_space_handler() doesn't access more than 64bit correctly. Found by kUBSan on Thinkpad X220. acpiec0 accessed 128bits from address 0xa0. The error message was: UBSan: Undefined Behavior in ../../../../dev/acpi/acpi_ec.c:672:32, shift exponent 64 is too large for 64-bit type 'long unsigned int' - KNF. - Make the case that width < 8 behave as the same as before. Pointed out by Joerg. - Change "switch" to "if" for simplify. To generate a diff of this commit: cvs rdiff -u -r1.75 -r1.75.6.1 src/sys/dev/acpi/acpi_ec.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/dev/acpi/acpi_ec.c diff -u src/sys/dev/acpi/acpi_ec.c:1.75 src/sys/dev/acpi/acpi_ec.c:1.75.6.1 --- src/sys/dev/acpi/acpi_ec.c:1.75 Sat Mar 11 08:26:23 2017 +++ src/sys/dev/acpi/acpi_ec.c Fri Aug 16 15:24:41 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: acpi_ec.c,v 1.75 2017/03/11 08:26:23 tsutsui Exp $ */ +/* $NetBSD: acpi_ec.c,v 1.75.6.1 2019/08/16 15:24:41 martin Exp $ */ /*- * Copyright (c) 2007 Joerg Sonnenberger <jo...@netbsd.org>. @@ -59,7 +59,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: acpi_ec.c,v 1.75 2017/03/11 08:26:23 tsutsui Exp $"); +__KERNEL_RCSID(0, "$NetBSD: acpi_ec.c,v 1.75.6.1 2019/08/16 15:24:41 martin Exp $"); #include <sys/param.h> #include <sys/callout.h> @@ -404,6 +404,7 @@ post_data_map: static bool acpiec_suspend(device_t dv, const pmf_qual_t *qual) { + acpiec_cold = true; return true; @@ -412,6 +413,7 @@ acpiec_suspend(device_t dv, const pmf_qu static bool acpiec_resume(device_t dv, const pmf_qual_t *qual) { + acpiec_cold = false; return true; @@ -454,9 +456,10 @@ acpiec_parse_gpe_package(device_t self, ACPI_FREE(p); return false; } - + if (p->Package.Count != 2) { - aprint_error_dev(self, "_GPE package does not contain 2 elements\n"); + aprint_error_dev(self, + "_GPE package does not contain 2 elements\n"); ACPI_FREE(p); return false; } @@ -511,6 +514,7 @@ static ACPI_STATUS acpiec_space_setup(ACPI_HANDLE region, uint32_t func, void *arg, void **region_arg) { + if (func == ACPI_REGION_DEACTIVATE) *region_arg = NULL; else @@ -528,9 +532,11 @@ acpiec_lock(device_t dv) mutex_enter(&sc->sc_access_mtx); if (sc->sc_need_global_lock) { - rv = AcpiAcquireGlobalLock(EC_LOCK_TIMEOUT, &sc->sc_global_lock); + rv = AcpiAcquireGlobalLock(EC_LOCK_TIMEOUT, + &sc->sc_global_lock); if (rv != AE_OK) { - aprint_error_dev(dv, "failed to acquire global lock: %s\n", + aprint_error_dev(dv, + "failed to acquire global lock: %s\n", AcpiFormatException(rv)); return; } @@ -546,7 +552,8 @@ acpiec_unlock(device_t dv) if (sc->sc_need_global_lock) { rv = AcpiReleaseGlobalLock(sc->sc_global_lock); if (rv != AE_OK) { - aprint_error_dev(dv, "failed to release global lock: %s\n", + aprint_error_dev(dv, + "failed to release global lock: %s\n", AcpiFormatException(rv)); } } @@ -587,7 +594,8 @@ acpiec_read(device_t dv, uint8_t addr, u } else if (cv_timedwait(&sc->sc_cv, &sc->sc_mtx, EC_CMD_TIMEOUT * hz)) { mutex_exit(&sc->sc_mtx); acpiec_unlock(dv); - aprint_error_dev(dv, "command takes over %d sec...\n", EC_CMD_TIMEOUT); + aprint_error_dev(dv, + "command takes over %d sec...\n", EC_CMD_TIMEOUT); return AE_ERROR; } @@ -634,7 +642,8 @@ acpiec_write(device_t dv, uint8_t addr, } else if (cv_timedwait(&sc->sc_cv, &sc->sc_mtx, EC_CMD_TIMEOUT * hz)) { mutex_exit(&sc->sc_mtx); acpiec_unlock(dv); - aprint_error_dev(dv, "command takes over %d sec...\n", EC_CMD_TIMEOUT); + aprint_error_dev(dv, + "command takes over %d sec...\n", EC_CMD_TIMEOUT); return AE_ERROR; } @@ -648,42 +657,36 @@ static ACPI_STATUS acpiec_space_handler(uint32_t func, ACPI_PHYSICAL_ADDRESS paddr, uint32_t width, ACPI_INTEGER *value, void *arg, void *region_arg) { - device_t dv; + device_t dv = arg; ACPI_STATUS rv; - uint8_t addr, reg; - unsigned int i; + uint8_t addr; + uint8_t *reg; + if ((func != ACPI_READ) && (func != ACPI_WRITE)) { + aprint_error("%s: invalid Address Space function called: %x\n", + device_xname(dv), (unsigned int)func); + return AE_BAD_PARAMETER; + } if (paddr > 0xff || width % 8 != 0 || value == NULL || arg == NULL || paddr + width / 8 > 0x100) return AE_BAD_PARAMETER; addr = paddr; - dv = arg; + reg = (uint8_t *)value; rv = AE_OK; - switch (func) { - case ACPI_READ: + if (func == ACPI_READ) *value = 0; - for (i = 0; i < width; i += 8, ++addr) { - rv = acpiec_read(dv, addr, ®); - if (rv != AE_OK) - break; - *value |= (ACPI_INTEGER)reg << i; - } - break; - case ACPI_WRITE: - for (i = 0; i < width; i += 8, ++addr) { - reg = (*value >>i) & 0xff; - rv = acpiec_write(dv, addr, reg); - if (rv != AE_OK) - break; - } - break; - default: - aprint_error("%s: invalid Address Space function called: %x\n", - device_xname(dv), (unsigned int)func); - return AE_BAD_PARAMETER; + + for (addr = paddr; addr < (paddr + width / 8); addr++, reg++) { + if (func == ACPI_READ) + rv = acpiec_read(dv, addr, reg); + else + rv = acpiec_write(dv, addr, *reg); + + if (rv != AE_OK) + break; } return rv; @@ -867,7 +870,8 @@ acpiec_bus_read(device_t dv, u_int addr, ACPI_STATUS acpiec_bus_write(device_t dv, u_int addr, ACPI_INTEGER val, int width) { - return acpiec_space_handler(ACPI_WRITE, addr, width * 8, &val, dv, NULL); + return acpiec_space_handler(ACPI_WRITE, addr, width * 8, &val, dv, + NULL); } ACPI_HANDLE