Skip to contentSkip to conversation

thinkpad_acpi: buffer overflow in fan_get_status()

PATCH In review Dan Carpenter 2 participants 2 messages Sep 1, 2012

Subsystems (2)
Kernel janitors x86 platform drivers
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 thinkpad_acpi: buffer overflow in fan_get_status()

The acpi_evalf() function modifies four bytes of data but in fan_get_status() we pass a pointer to u8. I have modified the function to use type checking now.

Signed-off-by Dan Carpenter <[email protected]>
drivers/platform/x86/thinkpad_acpi.c +7 -6
index df3016b..0735fb7 100644
@@ -545,7 +545,7 @@ TPACPI_HANDLE(hkey, ec, "\\_SB.HKEY",	/* 600e/x, 770e, 770x */
  */
 
 static int acpi_evalf(acpi_handle handle,
-		      void *res, char *method, char *fmt, ...)
+		      int *res, char *method, char *fmt, ...)
 {
 	char *fmt0 = fmt;
 	struct acpi_object_list params;

@@ -606,7 +606,7 @@ static int acpi_evalf(acpi_handle handle,
 		success = (status = AE_OK &&
 			   out_obj.type = ACPI_TYPE_INTEGER);
 		if (success && res)
-			*(int *)res = out_obj.integer.value;
+			*res = out_obj.integer.value;
 		break;
 	case 'v':		/* void */
 		success = status = AE_OK;

@@ -7386,17 +7386,18 @@ static int fan_get_status(u8 *status)
 	 * Add TPACPI_FAN_RD_ACPI_FANS ? */
 
 	switch (fan_status_access_mode) {
-	case TPACPI_FAN_RD_ACPI_GFAN:
+	case TPACPI_FAN_RD_ACPI_GFAN: {
 		/* 570, 600e/x, 770e, 770x */
+		int res;
 
-		if (unlikely(!acpi_evalf(gfan_handle, &s, NULL, "d")))
+		if (unlikely(!acpi_evalf(gfan_handle, &res, NULL, "d")))
 			return -EIO;
 
 		if (likely(status))
-			*status = s & 0x07;
+			*status = res & 0x07;
 
 		break;
-
+	}
 	case TPACPI_FAN_RD_TPEC:
 		/* all except 570, 600e/x, 770e, 770x */
 		if (unlikely(!acpi_ec_read(fan_status_offset, &s)))
Acked-by on v1 1/1
#1 lore raw
Quoted 3 lines · Dan Carpenter

The acpi_evalf() function modifies four bytes of data but in fan_get_status() we pass a pointer to u8. I have modified the function to use type checking now.

This makes the function unextensible to return other ACPI object types, but it can be changed back to void* later if required.

Quoted 1 line
Signed-off-by Dan Carpenter <[email protected]>
Acked-by Henrique de Moraes Holschuh <[email protected]>

Thanks for noticing this problem, fortunately it affects only _really_ ancient thinkpads, which are extremely rare.

Signature · 4 lines
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh