[v2,1/2] ACPI: battery: Fix buffer overread if not NUL-terminated

Message ID 20230119142115.38260-2-W_Armin@gmx.de
State New
Headers
Series ACPI: battery: Fix various string handling issues |

Commit Message

Armin Wolf Jan. 19, 2023, 2:21 p.m. UTC
  If a buffer containing ASCII characters is not NUL-terminated
(which is perfectly legal according to the ACPI specification),
the ACPI battery driver might not honor its length.
Fix this by limiting the amount of data to be copied to
the buffer length while also using strscpy() to make sure
that the resulting string is always NUL-terminated.
Also replace strncpy() vs strscpy().

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/acpi/battery.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

--
2.30.2
  

Patch

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index fb64bd217d82..0ec12a7dbcca 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -437,16 +437,25 @@  static int extract_package(struct acpi_battery *battery,
 		element = &package->package.elements[i];
 		if (offsets[i].mode) {
 			u8 *ptr = (u8 *)battery + offsets[i].offset;
+			u32 len = 32;

-			if (element->type == ACPI_TYPE_STRING ||
-			    element->type == ACPI_TYPE_BUFFER)
-				strscpy(ptr, element->string.pointer, 32);
-			else if (element->type == ACPI_TYPE_INTEGER) {
-				strncpy(ptr, (u8 *)&element->integer.value,
-					sizeof(u64));
-				ptr[sizeof(u64)] = 0;
-			} else
+			switch (element->type) {
+			case ACPI_TYPE_BUFFER:
+				if (len > element->buffer.length + 1)
+					len = element->buffer.length + 1;
+
+				fallthrough;
+			case ACPI_TYPE_STRING:
+				strscpy(ptr, element->string.pointer, len);
+
+				break;
+			case ACPI_TYPE_INTEGER:
+				strscpy(ptr, (u8 *)&element->integer.value, sizeof(u64) + 1);
+
+				break;
+			default:
 				*ptr = 0; /* don't have value */
+			}
 		} else {
 			int *x = (int *)((u8 *)battery + offsets[i].offset);
 			*x = (element->type == ACPI_TYPE_INTEGER) ?