Module Name:    src
Committed By:   martin
Date:           Fri Aug  9 16:13:36 UTC 2019

Modified Files:
        src/sys/dev/acpi [netbsd-9]: acpi_ec.c

Log Message:
Pull up following revision(s) (requested by msaitoh in ticket #38):

        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.20.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.20.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  9 16:13:35 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.20.1 2019/08/09 16:13:35 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.20.1 2019/08/09 16:13:35 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, &reg);
-			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

Reply via email to