Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/acpi - Fix a bug that acpiec_space_handler() doesn't...



details:   https://anonhg.NetBSD.org/src/rev/a2222a06962c
branches:  trunk
changeset: 453256:a2222a06962c
user:      msaitoh <msaitoh%NetBSD.org@localhost>
date:      Mon Aug 05 10:12:04 2019 +0000

description:
- 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.
The error message was:

diffstat:

 sys/dev/acpi/acpi_ec.c |  79 +++++++++++++++++++++++++++----------------------
 1 files changed, 44 insertions(+), 35 deletions(-)

diffs (177 lines):

diff -r 38197620ca1b -r a2222a06962c sys/dev/acpi/acpi_ec.c
--- a/sys/dev/acpi/acpi_ec.c    Mon Aug 05 10:09:35 2019 +0000
+++ b/sys/dev/acpi/acpi_ec.c    Mon Aug 05 10:12:04 2019 +0000
@@ -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.76 2019/08/05 10:12:04 msaitoh Exp $     */
 
 /*-
  * Copyright (c) 2007 Joerg Sonnenberger <joerg%NetBSD.org@localhost>.
@@ -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.76 2019/08/05 10:12:04 msaitoh Exp $");
 
 #include <sys/param.h>
 #include <sys/callout.h>
@@ -404,6 +404,7 @@
 static bool
 acpiec_suspend(device_t dv, const pmf_qual_t *qual)
 {
+
        acpiec_cold = true;
 
        return true;
@@ -412,6 +413,7 @@
 static bool
 acpiec_resume(device_t dv, const pmf_qual_t *qual)
 {
+
        acpiec_cold = false;
 
        return true;
@@ -454,9 +456,10 @@
                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 @@
 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 @@
        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 @@
        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 @@
        } 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 @@
        } 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,43 +657,42 @@
 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, &reg);
-                       if (rv != AE_OK)
-                               break;
-                       *value |= (ACPI_INTEGER)reg << i;
+
+       do {
+               switch (func) {
+               case ACPI_READ:
+                       rv = acpiec_read(dv, addr, reg);
+                       break;
+               case ACPI_WRITE:
+                       rv = acpiec_write(dv, addr, *reg);
+                       break;
                }
-               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;
-       }
+               if (rv != AE_OK)
+                       break;
+               addr++;
+               reg++;
+       } while (addr < (paddr + width / 8));
 
        return rv;
 }
@@ -867,7 +875,8 @@
 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



Home | Main Index | Thread Index | Old Index