All of lore.kernel.org
 help / color / mirror / Atom feed
From: Armin Wolf <W_Armin@gmx.de>
To: rafael@kernel.org, lenb@kernel.org
Cc: linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 2/4] ACPI: battery: Fix buffer overread if not NUL-terminated
Date: Sat, 14 Jan 2023 09:50:51 +0100	[thread overview]
Message-ID: <20230114085053.72059-3-W_Armin@gmx.de> (raw)
In-Reply-To: <20230114085053.72059-1-W_Armin@gmx.de>

If the buffer containing string data 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 use '\0' instead of a plain 0.

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

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index fb64bd217d82..9f6daa9f2010 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -438,15 +438,24 @@ static int extract_package(struct acpi_battery *battery,
 		if (offsets[i].mode) {
 			u8 *ptr = (u8 *)battery + offsets[i].offset;

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


  parent reply	other threads:[~2023-01-14  8:51 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-14  8:50 [PATCH 0/4] ACPI: battery: Fix various string handling issues Armin Wolf
2023-01-14  8:50 ` [PATCH 1/4] ACPI: battery: Fix missing NUL-termination with large strings Armin Wolf
2023-01-19 13:34   ` Rafael J. Wysocki
2023-01-14  8:50 ` Armin Wolf [this message]
2023-01-17 14:42   ` [PATCH 2/4] ACPI: battery: Fix buffer overread if not NUL-terminated Rafael J. Wysocki
2023-01-17 21:01     ` Armin Wolf
2023-01-18 15:00       ` Rafael J. Wysocki
2023-01-14  8:50 ` [PATCH 3/4] ACPI: battery: Replace strncpy() with strscpy() Armin Wolf
2023-01-17 14:44   ` Rafael J. Wysocki
2023-01-14  8:50 ` [PATCH 4/4] ACPI: battery: Increase maximum string length Armin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230114085053.72059-3-W_Armin@gmx.de \
    --to=w_armin@gmx.de \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.