[PATCH v2 0/3] platform/x86: hp-bioscfg: string buffer and password fixes

[PATCH v2 0/3] platform/x86: hp-bioscfg: string buffer and password fixes

[Muhammad Bilal]

This series addresses string buffer issues and password validation
in the hp-bioscfg driver.

Patch 1 fixes bounds checking, prescan OOB read, and character count
accounting in hp_get_string_from_buffer().

Patch 2 replaces the truncating manual conversion loop with a two-stage
conversion: utf16s_to_utf8s() into a scratch buffer followed by
string_escape_mem().

Patch 3 fixes validate_password_input() returning positive
INVALID_BIOS_AUTH (0x0E), which caused userspace writes of
out-of-range passwords to appear successful.

All patches tested on HP EliteBook 840 G2 (Fedora 44, Linux 7.2.7).

Changes in v2:
 - Split hp_get_string_from_buffer() fixes into two logical patches
   (bounds/OOB reads and UTF-8 conversion rewrite), as reviewed by
   Ilpo Järvinen.
 - Dropped lib/string_helpers patch: pass "\\\r\n\t" to string_escape_mem()
   with existing flags (ESCAPE_SPACE | ESCAPE_SPECIAL) to escape the four
   characters without altering quotes, as reviewed by Andy Shevchenko.
 - Added password validation errno fix as patch 3/3.

Muhammad Bilal (3):
  platform/x86: hp-bioscfg: fix OOB reads in hp_get_string_from_buffer()
  platform/x86: hp-bioscfg: fix non-ASCII truncation in
    hp_get_string_from_buffer()
  platform/x86: hp-bioscfg: return -ERANGE from
    validate_password_input()

 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c  | 77 ++++++-------------
 .../x86/hp/hp-bioscfg/passwdobj-attributes.c  |  6 +-
 2 files changed, 27 insertions(+), 56 deletions(-)

-- 
2.55.0

[PATCH v2 1/3] platform/x86: hp-bioscfg: fix OOB reads in hp_get_string_from_buffer()

[Muhammad Bilal]

The escape-prescan loop uses 'size' as both loop bound and
accumulator, so each match extends the bound and reads past src[]:

  for (i = 0; i < size; i++)
      if (src[i] == '\\' || ...) size++;

Snapshot the real char count into orig_size and loop against that.

Also fix two adjacent issues: the bounds check omits the 2-byte
length prefix already consumed, and utf16s_to_utf8s() receives the
byte count (src_size) instead of the u16-unit count (orig_size).

Tested on HP EliteBook 840 G2 (Fedora 44, Linux 7.2.7).

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v2:
 - Split bounds and prescan OOB fix into a separate patch, as
   reviewed by Ilpo Järvinen.

 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 24 ++++++++++++--------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
index 32b99a862082..ab1eff1deca1 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
@@ -60,6 +60,7 @@ int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_
 	u16 *src = (u16 *)*buffer;
 	u16 src_size;
 
+	u16 orig_size;
 	u16 size;
 	int i;
 	int conv_dst_size;
@@ -67,17 +68,20 @@ int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_
 	if (*buffer_size < sizeof(u16))
 		return -EINVAL;
 
-	src_size = *(src++);
-	/* size value in u16 chars */
-	size = src_size / sizeof(u16);
+	src_size = *src;
 
-	/* Ensure there is enough space remaining to read and convert
-	 * the string
+	/* Ensure there is enough space remaining for the length prefix
+	 * just read plus the string data it describes.
 	 */
-	if (*buffer_size < src_size)
+	if (*buffer_size < sizeof(u16) + src_size)
 		return -EINVAL;
 
-	for (i = 0; i < size; i++)
+	src++;
+	/* size value in u16 chars */
+	orig_size = src_size / sizeof(u16);
+	size = orig_size;
+
+	for (i = 0; i < orig_size; i++)
 		if (src[i] == '\\' ||
 		    src[i] == '\r' ||
 		    src[i] == '\n' ||
@@ -95,7 +99,7 @@ int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_
 	/*
 	 * convert from UTF-16 unicode to ASCII
 	 */
-	utf16s_to_utf8s(src, src_size, UTF16_HOST_ENDIAN, dst, conv_dst_size);
+	utf16s_to_utf8s(src, orig_size, UTF16_HOST_ENDIAN, dst, conv_dst_size);
 	dst[conv_dst_size] = 0;
 
 	for (i = 0; i < conv_dst_size; i++) {
@@ -121,8 +125,8 @@ int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_
 		src++;
 	}
 
-	*buffer = (u8 *)src;
-	*buffer_size -= size * sizeof(u16);
+	*buffer += sizeof(u16) + src_size;
+	*buffer_size -= sizeof(u16) + src_size;
 
 	return size;
 }
-- 
2.55.0

Re: [PATCH v2 1/3] platform/x86: hp-bioscfg: fix OOB reads in hp_get_string_from_buffer()

[Andy Shevchenko]

On Mon, Sep 28, 2026 at 11:03 PM Muhammad Bilal <meatuni001@gmail.com> wrote:
>
> The escape-prescan loop uses 'size' as both loop bound and
> accumulator, so each match extends the bound and reads past src[]:
>
>   for (i = 0; i < size; i++)
>       if (src[i] == '\\' || ...) size++;
>
> Snapshot the real char count into orig_size and loop against that.
>
> Also fix two adjacent issues: the bounds check omits the 2-byte
> length prefix already consumed, and utf16s_to_utf8s() receives the
> byte count (src_size) instead of the u16-unit count (orig_size).
>
> Tested on HP EliteBook 840 G2 (Fedora 44, Linux 7.2.7).

...

>  int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_

>         u16 *src = (u16 *)*buffer;
>         u16 src_size;

>

While at it, drop this blank line.

> +       u16 orig_size;
>         u16 size;
>         int i;
>         int conv_dst_size;

--
With Best Regards,
Andy Shevchenko

[PATCH v2 2/3] platform/x86: hp-bioscfg: fix non-ASCII truncation in hp_get_string_from_buffer()

[Muhammad Bilal]

utf16s_to_utf8s() writes into dst, but the manual loop that follows
re-walks src from the start and overwrites dst with truncating casts
(dst[i] = *src), discarding the UTF-8 output. Any code point above
U+007F produces garbage.

Replace both with a two-stage conversion: utf16s_to_utf8s() into a
scratch buffer, then string_escape_mem(ESCAPE_SPACE | ESCAPE_SPECIAL,
"\\\r\n\t") into dst. Passing the four characters to @only avoids
escaping quotes, leaving them for the existing strreplace() call.

Tested on HP EliteBook 840 G2 (Fedora 44, Linux 7.2.7).

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v2:
 - Split from the combined string buffer fix into its own patch, as
   reviewed by Ilpo Järvinen.
 - Drop lib/string_helpers patch: pass "\\\r\n\t" to string_escape_mem()
   with existing flags (ESCAPE_SPACE | ESCAPE_SPECIAL) to escape the four
   characters without modifying quotes, as reviewed by Andy Shevchenko.

 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 61 +++++---------------
 1 file changed, 14 insertions(+), 47 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
index ab1eff1deca1..fd3d54ee3a94 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
@@ -12,6 +12,7 @@
 #include <linux/kernel.h>
 #include <linux/printk.h>
 #include <linux/string.h>
+#include <linux/string_helpers.h>
 #include <linux/wmi.h>
 #include "bioscfg.h"
 #include "../../firmware_attributes_class.h"
@@ -60,10 +61,10 @@ int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_
 	u16 *src = (u16 *)*buffer;
 	u16 src_size;
 
+	char utf8_buf[MAX_BUFF_SIZE];
 	u16 orig_size;
-	u16 size;
-	int i;
-	int conv_dst_size;
+	int utf8_len;
+	int escaped_len;
 
 	if (*buffer_size < sizeof(u16))
 		return -EINVAL;
@@ -79,56 +80,22 @@ int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_
 	src++;
 	/* size value in u16 chars */
 	orig_size = src_size / sizeof(u16);
-	size = orig_size;
-
-	for (i = 0; i < orig_size; i++)
-		if (src[i] == '\\' ||
-		    src[i] == '\r' ||
-		    src[i] == '\n' ||
-		    src[i] == '\t')
-			size++;
 
-	/*
-	 * Conversion is limited to destination string max number of
-	 * bytes.
-	 */
-	conv_dst_size = size;
-	if (size >= dst_size)
-		conv_dst_size = dst_size - 1;
+	utf8_len = utf16s_to_utf8s(src, orig_size, UTF16_HOST_ENDIAN,
+				   utf8_buf, sizeof(utf8_buf));
 
-	/*
-	 * convert from UTF-16 unicode to ASCII
-	 */
-	utf16s_to_utf8s(src, orig_size, UTF16_HOST_ENDIAN, dst, conv_dst_size);
-	dst[conv_dst_size] = 0;
-
-	for (i = 0; i < conv_dst_size; i++) {
-		if (*src == '\\' ||
-		    *src == '\r' ||
-		    *src == '\n' ||
-		    *src == '\t') {
-			dst[i++] = '\\';
-			if (i == conv_dst_size)
-				break;
-		}
-
-		if (*src == '\r')
-			dst[i] = 'r';
-		else if (*src == '\n')
-			dst[i] = 'n';
-		else if (*src == '\t')
-			dst[i] = 't';
-		else if (*src == '"')
-			dst[i] = '\'';
-		else
-			dst[i] = *src;
-		src++;
-	}
+	escaped_len = string_escape_mem(utf8_buf, utf8_len, dst, dst_size - 1,
+					ESCAPE_SPACE | ESCAPE_SPECIAL,
+					"\\\r\n\t");
+	if (escaped_len > dst_size - 1)
+		escaped_len = dst_size - 1;
+	dst[escaped_len] = '\0';
+	strreplace(dst, '"', '\'');
 
 	*buffer += sizeof(u16) + src_size;
 	*buffer_size -= sizeof(u16) + src_size;
 
-	return size;
+	return escaped_len;
 }
 
 int hp_get_common_data_from_buffer(u8 **buffer_ptr, u32 *buffer_size,
-- 
2.55.0

Re: [PATCH v2 2/3] platform/x86: hp-bioscfg: fix non-ASCII truncation in hp_get_string_from_buffer()

[Andy Shevchenko]

On Mon, Sep 28, 2026 at 11:03 PM Muhammad Bilal <meatuni001@gmail.com> wrote:
>
> utf16s_to_utf8s() writes into dst, but the manual loop that follows
> re-walks src from the start and overwrites dst with truncating casts
> (dst[i] = *src), discarding the UTF-8 output. Any code point above
> U+007F produces garbage.
>
> Replace both with a two-stage conversion: utf16s_to_utf8s() into a
> scratch buffer, then string_escape_mem(ESCAPE_SPACE | ESCAPE_SPECIAL,
> "\\\r\n\t") into dst. Passing the four characters to @only avoids
> escaping quotes, leaving them for the existing strreplace() call.
>
> Tested on HP EliteBook 840 G2 (Fedora 44, Linux 7.2.7).
>
> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Wrong tag. I have not suggested the (main) idea behind this change.
You're not supposed to arbitrarily add some tags without respective
consent.

> Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>

...

>  #include <linux/string.h>
> +#include <linux/string_helpers.h>

You can replace string.h with string_helpers.h as the latter implies the former.

...

>         u16 *src = (u16 *)*buffer;
>         u16 src_size;
>
> +       char utf8_buf[MAX_BUFF_SIZE];

Try to preserve reversed xmas tree order.

>         u16 orig_size;
> -       u16 size;
> -       int i;
> -       int conv_dst_size;
> +       int utf8_len;
> +       int escaped_len;

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 3/3] platform/x86: hp-bioscfg: return -ERANGE from validate_password_input()

[Muhammad Bilal]

validate_password_input() returns INVALID_BIOS_AUTH (0x0E, positive)
on rejection. store_password_instance() skips the store on nonzero
ret, but its final "return ret < 0 ? ret : count" treats the positive
value as success, so userspace sees count instead of an error.

Return -ERANGE, matching the sibling validators for string,
enumeration, and integer attribute types.

Tested on HP EliteBook 840 G2 (Fedora 44, Linux 7.2.7): writing 999
bytes to current_password returned 999 with no error before this fix.

Fixes: 8646a3b5ee3a ("platform/x86: hp-bioscfg: passwdobj-attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v2:
 - New patch in this series.

 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index 9b989ef756ea..b1c687fb4700 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -70,12 +70,12 @@ static int validate_password_input(int instance_id, const char *buf)
 		length--;
 
 	if (length > MAX_PASSWD_SIZE)
-		return INVALID_BIOS_AUTH;
+		return -ERANGE;
 
 	if (password_data->min_password_length > length ||
 	    password_data->max_password_length < length)
-		return INVALID_BIOS_AUTH;
-	return SUCCESS;
+		return -ERANGE;
+	return 0;
 }
 
 ATTRIBUTE_N_PROPERTY_SHOW(is_enabled, password);
-- 
2.55.0

Re: [PATCH v2 3/3] platform/x86: hp-bioscfg: return -ERANGE from validate_password_input()

[Andy Shevchenko]

On Mon, Sep 28, 2026 at 11:03 PM Muhammad Bilal <meatuni001@gmail.com> wrote:
>
> validate_password_input() returns INVALID_BIOS_AUTH (0x0E, positive)
> on rejection. store_password_instance() skips the store on nonzero
> ret, but its final "return ret < 0 ? ret : count" treats the positive
> value as success, so userspace sees count instead of an error.
>
> Return -ERANGE, matching the sibling validators for string,
> enumeration, and integer attribute types.
>
> Tested on HP EliteBook 840 G2 (Fedora 44, Linux 7.2.7): writing 999
> bytes to current_password returned 999 with no error before this fix.

...

>         if (length > MAX_PASSWD_SIZE)
> -               return INVALID_BIOS_AUTH;
> +               return -ERANGE;

Nope, ERANGE is for integer values. Here is definitely about the size
of the input string. We have E2BIG for such cases.

>         if (password_data->min_password_length > length ||
>             password_data->max_password_length < length)
> -               return INVALID_BIOS_AUTH;
> -       return SUCCESS;
> +               return -ERANGE;

Ditto.

> +       return 0;
>  }

-- 
With Best Regards,
Andy Shevchenko