[PATCH v4 0/9] platform/x86: hp-bioscfg: buffer handling fixes

[PATCH v4 0/9] platform/x86: hp-bioscfg: buffer handling fixes

[Muhammad Bilal]

Buffer handling fixes for hp-bioscfg.

Patches 1-2 fix hp_get_string_from_buffer(), 3-5 the password
handling, 6 the sysfs store paths, 7-9 SPM and integer parsing.

Changes in v4:
 - Add patches 4-9

Changes in v3:
 - Drop Suggested-by: Andy Shevchenko from patch 2, tidy its includes
   and declarations (Andy)
 - Return -E2BIG instead of -ERANGE in patch 3 (Andy)
 - Drop a blank line in the declarations in patch 1 (Andy)

Changes in v2:
 - Split the hp_get_string_from_buffer() fix in two (Ilpo)
 - Drop the lib/string_helpers patch, use @only of string_escape_mem()
   instead (Andy)
 - Add patch 3

Muhammad Bilal (9):
  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 -E2BIG from validate_password_input()
  platform/x86: hp-bioscfg: allow clearing current_password
  platform/x86: hp-bioscfg: fix off-by-one in password length check
  platform/x86: hp-bioscfg: fix heap OOB with embedded NUL in store
    paths
  platform/x86: hp-bioscfg: validate SPM state returned by firmware
  platform/x86: hp-bioscfg: NUL-terminate the SPM auth token
  platform/x86: hp-bioscfg: fix oops on short integer attribute buffer

 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c  | 79 ++++++-------------
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.h  |  2 +-
 .../x86/hp/hp-bioscfg/int-attributes.c        |  7 +-
 .../x86/hp/hp-bioscfg/passwdobj-attributes.c  | 20 +++--
 .../x86/hp/hp-bioscfg/spmobj-attributes.c     |  8 +-
 5 files changed, 51 insertions(+), 65 deletions(-)

-- 
2.43.0

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

[Muhammad Bilal]

The escape prescan loop uses 'size' as both its bound and its
accumulator, so each escape character found extends the loop and
reads past the end of src[].

Use the original u16 count as the loop bound. Also include the 2-byte
length prefix in the bounds check, pass the u16 count instead of the
byte count to utf16s_to_utf8s(), and advance the buffer by the bytes
actually consumed instead of the escape-inflated count.

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

Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
Cc: stable@vger.kernel.org
Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v3:
 - Drop a blank line in the declarations (Andy)
 - Mention the buffer advance in the changelog

Changes in v2:
 - Split the bounds and prescan fix out of the conversion rewrite (Ilpo)

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

diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
index 309634c1c..74a90867e 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
@@ -54,8 +54,8 @@ int hp_get_integer_from_buffer(u8 **buffer, u32 *buffer_size, u32 *integer)
 int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_size)
 {
 	u16 *src = (u16 *)*buffer;
+	u16 orig_size;
 	u16 src_size;
-
 	u16 size;
 	int i;
 	int conv_dst_size;
@@ -63,17 +63,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' ||
@@ -91,7 +94,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++) {
@@ -117,8 +120,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.43.0

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

[Andy Shevchenko]

On Sat, Oct 03, 2026 at 12:14:26AM +0500, Muhammad Bilal wrote:
> The escape prescan loop uses 'size' as both its bound and its
> accumulator, so each escape character found extends the loop and
> reads past the end of src[].
> 
> Use the original u16 count as the loop bound. Also include the 2-byte
> length prefix in the bounds check, pass the u16 count instead of the
> byte count to utf16s_to_utf8s(), and advance the buffer by the bytes
> actually consumed instead of the escape-inflated count.
> 
> Tested on HP EliteBook 840 G2 (Fedora 44, Linux 7.2.7).

...

> -	/* 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.
>  	 */

/*
 * While at it, fix the comment style for
 * multi-line comments. You can use this example.
 */

-- 
With Best Regards,
Andy Shevchenko

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

[Muhammad Bilal]

utf16s_to_utf8s() writes into dst, but the loop that follows
overwrites dst from the raw UTF-16 code units with truncating casts,
so any character above U+007F is mangled. For example, U+00E9 is
stored as 0xe9 instead of 0xc3 0xa9.

Convert into a scratch buffer first, then escape '\\', '\r', '\n' and
'\t' into dst with string_escape_mem(). Quotes are not escaped, so the
existing strreplace() still handles them.

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

Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
Cc: stable@vger.kernel.org
Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v3:
 - Drop Suggested-by: Andy Shevchenko
 - Use string_helpers.h instead of string.h, keep reverse xmas tree
   order of declarations (Andy)

Changes in v2:
 - Split out of the combined fix (Ilpo)
 - Use @only of string_escape_mem() instead of a new flag (Andy)

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

diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
index 74a90867e..e468995f2 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
@@ -11,7 +11,7 @@
 #include <linux/module.h>
 #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"
@@ -53,12 +53,12 @@ int hp_get_integer_from_buffer(u8 **buffer, u32 *buffer_size, u32 *integer)
 
 int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_size)
 {
+	char utf8_buf[MAX_BUFF_SIZE];
 	u16 *src = (u16 *)*buffer;
+	int escaped_len;
 	u16 orig_size;
 	u16 src_size;
-	u16 size;
-	int i;
-	int conv_dst_size;
+	int utf8_len;
 
 	if (*buffer_size < sizeof(u16))
 		return -EINVAL;
@@ -74,56 +74,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.43.0

[PATCH v4 3/9] platform/x86: hp-bioscfg: return -E2BIG from validate_password_input()

[Muhammad Bilal]

validate_password_input() returns the positive INVALID_BIOS_AUTH 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 -E2BIG for an invalid password length.

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

Fixes: 8646a3b5ee3a ("platform/x86: hp-bioscfg: passwdobj-attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v3:
 - Return -E2BIG instead of -ERANGE (Andy)

Changes in v2:
 - New patch

 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c | 7 ++++---
 1 file changed, 4 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 a9e178637..3f4c45d9b 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -70,12 +70,13 @@ static int validate_password_input(int instance_id, const char *buf)
 		length--;
 
 	if (length > MAX_PASSWD_SIZE)
-		return INVALID_BIOS_AUTH;
+		return -E2BIG;
 
 	if (password_data->min_password_length > length ||
 	    password_data->max_password_length < length)
-		return INVALID_BIOS_AUTH;
-	return SUCCESS;
+		return -E2BIG;
+
+	return 0;
 }
 
 ATTRIBUTE_N_PROPERTY_SHOW(is_enabled, password);
-- 
2.43.0

[PATCH v4 4/9] platform/x86: hp-bioscfg: allow clearing current_password

[Muhammad Bilal]

The ABI documents echo "" > current_password as the way to end a
session, but validate_password_input() rejects an empty string when
min_password_length is nonzero. The password stays cached, and since
the previous patch the write also fails with -E2BIG.

Skip the length check for an empty current_password so the write
clears it.

Before the previous patch the write returned 1 on an HP EliteBook 840
G2 (min_password_length is 8) but cleared nothing.

Compile tested only.

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

 .../platform/x86/hp/hp-bioscfg/passwdobj-attributes.c    | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index 3f4c45d9b..f0551b455 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -60,7 +60,8 @@ int hp_get_password_instance_for_type(const char *name)
 	return -EINVAL;
 }
 
-static int validate_password_input(int instance_id, const char *buf)
+static int validate_password_input(int instance_id, const char *buf,
+				   bool is_current)
 {
 	int length;
 	struct password_data *password_data = &bioscfg_drv.password_data[instance_id];
@@ -69,6 +70,10 @@ static int validate_password_input(int instance_id, const char *buf)
 	if (length > 0 && buf[length - 1] == '\n')
 		length--;
 
+	/* An empty current_password clears the session password */
+	if (is_current && !length)
+		return 0;
+
 	if (length > MAX_PASSWD_SIZE)
 		return -E2BIG;
 
@@ -97,7 +102,7 @@ static int store_password_instance(struct kobject *kobj, const char *buf,
 		id = get_password_instance_id(kobj);
 
 		if (id >= 0)
-			ret = validate_password_input(id, buf_cp);
+			ret = validate_password_input(id, buf_cp, is_current);
 	}
 
 	if (!ret) {
-- 
2.43.0

Re: [PATCH v4 4/9] platform/x86: hp-bioscfg: allow clearing current_password

[Ilpo Järvinen]

On Sat, 3 Oct 2026, Muhammad Bilal wrote:

> The ABI documents echo "" > current_password as the way to end a
> session, but validate_password_input() rejects an empty string when
> min_password_length is nonzero. The password stays cached, and since
> the previous patch the write also fails with -E2BIG.

After I apply a patch, it's no longer a "patch", so the wording should 
anticipate that.

> Skip the length check for an empty current_password so the write
> clears it.
> 
> Before the previous patch the write returned 1 on an HP EliteBook 840
> G2 (min_password_length is 8) but cleared nothing.
> 
> Compile tested only.
> 
> Fixes: 8646a3b5ee3a ("platform/x86: hp-bioscfg: passwdobj-attributes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
> Changes in v4:
>  - New patch
> 
>  .../platform/x86/hp/hp-bioscfg/passwdobj-attributes.c    | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> index 3f4c45d9b..f0551b455 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> @@ -60,7 +60,8 @@ int hp_get_password_instance_for_type(const char *name)
>  	return -EINVAL;
>  }
>  
> -static int validate_password_input(int instance_id, const char *buf)
> +static int validate_password_input(int instance_id, const char *buf,
> +				   bool is_current)
>  {
>  	int length;
>  	struct password_data *password_data = &bioscfg_drv.password_data[instance_id];
> @@ -69,6 +70,10 @@ static int validate_password_input(int instance_id, const char *buf)
>  	if (length > 0 && buf[length - 1] == '\n')
>  		length--;
>  
> +	/* An empty current_password clears the session password */
> +	if (is_current && !length)
> +		return 0;
> +
>  	if (length > MAX_PASSWD_SIZE)
>  		return -E2BIG;
>  
> @@ -97,7 +102,7 @@ static int store_password_instance(struct kobject *kobj, const char *buf,
>  		id = get_password_instance_id(kobj);
>  
>  		if (id >= 0)
> -			ret = validate_password_input(id, buf_cp);
> +			ret = validate_password_input(id, buf_cp, is_current);
>  	}
>  
>  	if (!ret) {
> 

-- 
 i.

[PATCH v4 5/9] platform/x86: hp-bioscfg: fix off-by-one in password length check

[Muhammad Bilal]

current_password and new_password hold MAX_PASSWD_SIZE bytes including
the NUL, but validate_password_input() accepts a password of exactly
MAX_PASSWD_SIZE characters when the firmware limits allow it.
strscpy() then truncates it and fails with -E2BIG, which
store_password_instance() ignores, so the write reports success with a
truncated password stored.

For example, with a max_password_length of 64 or more, writing 64
characters succeeds but only 63 are stored.

Reject lengths of MAX_PASSWD_SIZE or more.

Compile tested only.

Fixes: 8646a3b5ee3a ("platform/x86: hp-bioscfg: passwdobj-attributes")
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v4:
 - New patch

 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index f0551b455..a2f50ecbe 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -74,7 +74,7 @@ static int validate_password_input(int instance_id, const char *buf,
 	if (is_current && !length)
 		return 0;
 
-	if (length > MAX_PASSWD_SIZE)
+	if (length >= MAX_PASSWD_SIZE)
 		return -E2BIG;
 
 	if (password_data->min_password_length > length ||
-- 
2.43.0

[PATCH v4 6/9] platform/x86: hp-bioscfg: fix heap OOB with embedded NUL in store paths

[Muhammad Bilal]

The store handlers copy the input with kstrdup(), which stops at the
first NUL, but pass the full write size to
hp_enforce_single_line_input(). With an embedded NUL the copy is
shorter than count, so the helper reads, and can write one byte, past
the allocation.

Writing "A\0" followed by 4093 bytes of "B" to current_password makes
memchr() scan 4093 bytes past a 2-byte copy. On an HP EliteBook 840 G2
running Linux 7.2.7 all 100 such writes fail with -EINVAL although the
input has no newline, so memchr() matched one in the heap. A userspace
replica under ASan reports the same 4095 byte read, 0 bytes after the
2-byte region.

Use kmemdup_nul() so the copy is always count + 1 bytes long.

Compile tested only.

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

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

diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
index ac57d6eab..77cb9cac1 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
@@ -326,7 +326,7 @@ enum hp_wmi_data_elements {
 		int i;							\
 		int ret = -EIO;						\
 									\
-		attr_value = kstrdup(buf, GFP_KERNEL);			\
+		attr_value = kmemdup_nul(buf, count, GFP_KERNEL);	\
 		if (!attr_value)					\
 			return -ENOMEM;					\
 									\
diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index a2f50ecbe..6c123d7a5 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -93,7 +93,7 @@ static int store_password_instance(struct kobject *kobj, const char *buf,
 	char *buf_cp;
 	int id, ret = 0;
 
-	buf_cp = kstrdup(buf, GFP_KERNEL);
+	buf_cp = kmemdup_nul(buf, count, GFP_KERNEL);
 	if (!buf_cp)
 		return -ENOMEM;
 
-- 
2.43.0

Re: [PATCH v4 6/9] platform/x86: hp-bioscfg: fix heap OOB with embedded NUL in store paths

[Ilpo Järvinen]

On Sat, 3 Oct 2026, Muhammad Bilal wrote:

> The store handlers copy the input with kstrdup(), which stops at the
> first NUL, but pass the full write size to
> hp_enforce_single_line_input(). With an embedded NUL the copy is
> shorter than count, so the helper reads, and can write one byte, past
> the allocation.
> 
> Writing "A\0" followed by 4093 bytes of "B" to current_password makes
> memchr() scan 4093 bytes past a 2-byte copy. On an HP EliteBook 840 G2
> running Linux 7.2.7 all 100 such writes fail with -EINVAL although the
> input has no newline, so memchr() matched one in the heap. A userspace
> replica under ASan reports the same 4095 byte read, 0 bytes after the
> 2-byte region.
> 
> Use kmemdup_nul() so the copy is always count + 1 bytes long.
> 
> Compile tested only.
> 
> Fixes: 5f94f181ca25 ("platform/x86: hp-bioscfg: bioscfg-h")
> Fixes: 8646a3b5ee3a ("platform/x86: hp-bioscfg: passwdobj-attributes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>

I don't understand this change.

What is the usecase for using the input beyond the first NUL?

> ---
> Changes in v4:
>  - New patch
> 
>  drivers/platform/x86/hp/hp-bioscfg/bioscfg.h              | 2 +-
>  drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
> index ac57d6eab..77cb9cac1 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
> +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
> @@ -326,7 +326,7 @@ enum hp_wmi_data_elements {
>  		int i;							\
>  		int ret = -EIO;						\
>  									\
> -		attr_value = kstrdup(buf, GFP_KERNEL);			\
> +		attr_value = kmemdup_nul(buf, count, GFP_KERNEL);	\
>  		if (!attr_value)					\
>  			return -ENOMEM;					\
>  									\
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> index a2f50ecbe..6c123d7a5 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> @@ -93,7 +93,7 @@ static int store_password_instance(struct kobject *kobj, const char *buf,
>  	char *buf_cp;
>  	int id, ret = 0;
>  
> -	buf_cp = kstrdup(buf, GFP_KERNEL);
> +	buf_cp = kmemdup_nul(buf, count, GFP_KERNEL);
>  	if (!buf_cp)
>  		return -ENOMEM;
>  
> 

-- 
 i.

[PATCH v4 7/9] platform/x86: hp-bioscfg: validate SPM state returned by firmware

[Muhammad Bilal]

status_show() and update_spm_state() use the state byte from firmware
to index 3-entry string arrays without a range check. A value above 2
indexes past the arrays, and the result is printed with %s by the
world-readable status and key_mechanism files.

For example, a state of 3 makes status treat whatever follows
spm_state_types[] as a string pointer.

Return -EIO for an out of range state.

Compile tested only.

Fixes: b2715aa2e135 ("platform/x86: hp-bioscfg: spmobj-attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v4:
 - New patch

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

diff --git a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
index 4d94e48c1..f0eb5c445 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
@@ -118,6 +118,9 @@ static ssize_t update_spm_state(void)
 	if (ret < 0)
 		return ret;
 
+	if (data.state >= ARRAY_SIZE(spm_mechanism_types))
+		return -EIO;
+
 	bioscfg_drv.spm_data.mechanism = data.state;
 	if (bioscfg_drv.spm_data.mechanism)
 		bioscfg_drv.spm_data.is_enabled = 1;
@@ -153,6 +156,9 @@ static ssize_t status_show(struct kobject *kobj, struct kobj_attribute
 	if (ret < 0)
 		return ret;
 
+	if (data.state >= ARRAY_SIZE(spm_state_types))
+		return -EIO;
+
 	/*
 	 * 'status' is a read-only file that returns ASCII text in
 	 * JSON format reporting the status information.
-- 
2.43.0

Re: [PATCH v4 7/9] platform/x86: hp-bioscfg: validate SPM state returned by firmware

[Ilpo Järvinen]

On Sat, 3 Oct 2026, Muhammad Bilal wrote:

> status_show() and update_spm_state() use the state byte from firmware
> to index 3-entry string arrays without a range check. A value above 2
> indexes past the arrays, and the result is printed with %s by the
> world-readable status and key_mechanism files.
> 
> For example, a state of 3 makes status treat whatever follows
> spm_state_types[] as a string pointer.
> 
> Return -EIO for an out of range state.
> 
> Compile tested only.
> 
> Fixes: b2715aa2e135 ("platform/x86: hp-bioscfg: spmobj-attributes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
> Changes in v4:
>  - New patch
> 
>  drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
> index 4d94e48c1..f0eb5c445 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
> @@ -118,6 +118,9 @@ static ssize_t update_spm_state(void)
>  	if (ret < 0)
>  		return ret;
>  
> +	if (data.state >= ARRAY_SIZE(spm_mechanism_types))

Please add the include for this (to the .c file, I know it includes 
bioscfg.h as is but common include handing all includes for a multi-file 
driver is undesirable practice).

> +		return -EIO;
> +
>  	bioscfg_drv.spm_data.mechanism = data.state;
>  	if (bioscfg_drv.spm_data.mechanism)
>  		bioscfg_drv.spm_data.is_enabled = 1;
> @@ -153,6 +156,9 @@ static ssize_t status_show(struct kobject *kobj, struct kobj_attribute
>  	if (ret < 0)
>  		return ret;
>  
> +	if (data.state >= ARRAY_SIZE(spm_state_types))
> +		return -EIO;
> +
>  	/*
>  	 * 'status' is a read-only file that returns ASCII text in
>  	 * JSON format reporting the status information.
> 

-- 
 i.

[PATCH v4 8/9] platform/x86: hp-bioscfg: NUL-terminate the SPM auth token

[Muhammad Bilal]

auth_token_store() copies the token with kmemdup(), which does not NUL
terminate it, but hp_calculate_security_buffer() and
hp_populate_security_buffer() treat it as a C string and read past the
allocation.

A lone newline (echo > auth_token) is worse: kmemdup() of 0 bytes
returns ZERO_SIZE_PTR, which passes the NULL checks, so a later
attribute write calls strlen() on address 0x10.

Use kmemdup_nul(), which allocates one extra byte for the terminator
and never returns ZERO_SIZE_PTR.

Compile tested only.

Fixes: b2715aa2e135 ("platform/x86: hp-bioscfg: spmobj-attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v4:
 - New patch

 drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
index f0eb5c445..19f0f9f16 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
@@ -316,7 +316,7 @@ static ssize_t auth_token_store(struct kobject *kobj,
 		length--;
 
 	/* allocate space and copy current auth token */
-	bioscfg_drv.spm_data.auth_token = kmemdup(buf, length, GFP_KERNEL);
+	bioscfg_drv.spm_data.auth_token = kmemdup_nul(buf, length, GFP_KERNEL);
 	if (!bioscfg_drv.spm_data.auth_token) {
 		ret = -ENOMEM;
 		goto exit_token;
-- 
2.43.0

Re: [PATCH v4 8/9] platform/x86: hp-bioscfg: NUL-terminate the SPM auth token

[Ilpo Järvinen]

On Sat, 3 Oct 2026, Muhammad Bilal wrote:

> auth_token_store() copies the token with kmemdup(), which does not NUL
> terminate it, but hp_calculate_security_buffer() and
> hp_populate_security_buffer() treat it as a C string and read past the
> allocation.

Again here.

If something is treated as C string later, how is it valid to prepare it 
with kmemdup_nul()? I just don't follow that logic.

Either something is a string or it isn't, which way this is?

> A lone newline (echo > auth_token) is worse: kmemdup() of 0 bytes
> returns ZERO_SIZE_PTR, which passes the NULL checks, so a later
> attribute write calls strlen() on address 0x10.
> 
> Use kmemdup_nul(), which allocates one extra byte for the terminator
> and never returns ZERO_SIZE_PTR.
> 
> Compile tested only.
> 
> Fixes: b2715aa2e135 ("platform/x86: hp-bioscfg: spmobj-attributes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
> Changes in v4:
>  - New patch
> 
>  drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
> index f0eb5c445..19f0f9f16 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
> @@ -316,7 +316,7 @@ static ssize_t auth_token_store(struct kobject *kobj,
>  		length--;
>  
>  	/* allocate space and copy current auth token */
> -	bioscfg_drv.spm_data.auth_token = kmemdup(buf, length, GFP_KERNEL);
> +	bioscfg_drv.spm_data.auth_token = kmemdup_nul(buf, length, GFP_KERNEL);
>  	if (!bioscfg_drv.spm_data.auth_token) {
>  		ret = -ENOMEM;
>  		goto exit_token;
> 

-- 
 i.

[PATCH v4 9/9] platform/x86: hp-bioscfg: fix oops on short integer attribute buffer

[Muhammad Bilal]

hp_populate_integer_elements_from_buffer() ignores the return value of
hp_get_string_from_buffer(). When fewer than 2 bytes are left in the
buffer, dst_size is 0, kcalloc() returns ZERO_SIZE_PTR and
hp_get_string_from_buffer() fails without writing to it, so
kstrtoint() faults on address 0x10.

Return the error instead.

Compile tested only.

Fixes: 6f2c06d5a467 ("platform/x86: hp-bioscfg: int-attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v4:
 - New patch

 drivers/platform/x86/hp/hp-bioscfg/int-attributes.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
index a27907066..14317b7fd 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
@@ -331,7 +331,12 @@ static int hp_populate_integer_elements_from_buffer(u8 *buffer_ptr, u32 *buffer_
 	// VALUE:
 	integer_data->current_value = 0;
 
-	hp_get_string_from_buffer(&buffer_ptr, buffer_size, dst, dst_size);
+	ret = hp_get_string_from_buffer(&buffer_ptr, buffer_size, dst, dst_size);
+	if (ret < 0) {
+		kfree(dst);
+		return ret;
+	}
+
 	ret = kstrtoint(dst, 10, &integer_data->current_value);
 	if (ret)
 		pr_warn("Unable to convert string to integer: %s\n", dst);
-- 
2.43.0