[PATCH v2 0/9] platform/x86: hp-bioscfg: fix multiple memory safety bugs and parsing errors

[PATCH v2 0/9] platform/x86: hp-bioscfg: fix multiple memory safety bugs and parsing errors

[Muhammad Bilal]

This series fixes several bugs in the hp-bioscfg driver:

1. Memory safety issues (patches 1-6):
   - Off-by-one NUL terminator write in hp_get_string_from_buffer()
   - Heap OOB read in sk_store()/kek_store() when passing original
     count instead of trimmed length to hp_wmi_perform_query()
   - Heap OOB read on empty password write in validate_password_input()
   - 16-byte heap overflow in hp_calculate_security_buffer() for empty
     authentication tokens
   - Off-by-one heap OOB write in audit_log_entries_show() when more
     than 256 log entries are reported
   - Missing bounds check in the PSWD_ENCODINGS parsing loop

2. Logic bugs (patches 7-8):
   - new_password_store() incorrectly passing is_current=true, causing
     writes to land in current_password instead
   - ORD_LIST_ELEMENTS case using a stale NULL str_value pointer
     instead of converting the current ACPI element directly

3. ACPI package element index tracking (patch 9):
   - Five attribute-type parsers (enum, int, string, order-list,
     passwd) all share the same defect: multi-element array cases
     consume "size" consecutive elements but the outer loop only
     advances by one, causing the next iteration to misread a
     leftover array entry as the next property and abort with -EIO

Tested on HP EliteBook 840 G2 (BIOS M71 Ver. 01.31), kernel 7.2.0-rc5+,
with CONFIG_KASAN_GENERIC=y and CONFIG_SLUB_DEBUG=y.

This series applies on top of:
  commit ea4d8f8ba283 ("platform/x86: hp-bioscfg: fix
  slab-out-of-bounds write in hp_convert_hexstr_to_str")

Changes in v2:
  - Squash patches 9-13 from v1 into a single patch (now patch 9),
    as the fix is identical across all five attribute-type parsers.
    Requested by Ilpo Järvinen.

v1: https://lore.kernel.org/all/20260803143037.93105-1-meatuni001@gmail.com/

Muhammad Bilal (9):
  platform/x86: hp-bioscfg: fix off-by-one write in
    hp_get_string_from_buffer
  platform/x86: hp-bioscfg: fix heap OOB read in sk_store and kek_store
  platform/x86: hp-bioscfg: fix heap OOB read on empty password write
  platform/x86: hp-bioscfg: fix 16-byte heap overflow for empty auth
    token
  platform/x86: hp-bioscfg: fix off-by-one heap OOB write in
    audit_log_entries_show
  platform/x86: hp-bioscfg: add missing bounds check in PSWD_ENCODINGS
    loop
  platform/x86: hp-bioscfg: fix new_password_store overwriting
    current_password
  platform/x86: hp-bioscfg: fix ORD_LIST_ELEMENTS never being parsed
  platform/x86: hp-bioscfg: advance elem past consumed array elements

 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c        |  2 +-
 .../platform/x86/hp/hp-bioscfg/enum-attributes.c    |  4 ++++
 drivers/platform/x86/hp/hp-bioscfg/int-attributes.c |  2 ++
 .../x86/hp/hp-bioscfg/order-list-attributes.c       |  6 +++++-
 .../x86/hp/hp-bioscfg/passwdobj-attributes.c        | 13 +++++++++++--
 .../platform/x86/hp/hp-bioscfg/spmobj-attributes.c  |  6 +++---
 .../platform/x86/hp/hp-bioscfg/string-attributes.c  |  2 ++
 .../x86/hp/hp-bioscfg/surestart-attributes.c        |  2 +-
 8 files changed, 29 insertions(+), 8 deletions(-)

-- 
2.55.0

[PATCH v2 1/9] platform/x86: hp-bioscfg: fix off-by-one write in hp_get_string_from_buffer

[Muhammad Bilal]

hp_get_string_from_buffer() clamps the converted string length against
the destination buffer size with "size > dst_size", so when the
converted length is exactly equal to dst_size, conv_dst_size is left
at dst_size and the unconditional NUL terminator write

	dst[conv_dst_size] = 0;

lands one byte past the destination buffer. This is the same shape of
bug as the previously fixed off-by-one in hp_convert_hexstr_to_str():
the buffer is sized correctly for the content, but the terminator
write is never checked against that size.

Fix by changing the comparison to ">=" so conv_dst_size is always left
with room for the terminator.

All fixed-size destinations that reach this function (path[512],
current_value[512], current_password/current_value[64], and the
per-entry buffers in encodings[][512] and prerequisites[][512]) are
affected.

Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
index 2bf57e6eade4..0edc6e7cfa9a 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
@@ -85,7 +85,7 @@ int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_
 	 * bytes.
 	 */
 	conv_dst_size = size;
-	if (size > dst_size)
+	if (size >= dst_size)
 		conv_dst_size = dst_size - 1;
 
 	/*
-- 
2.55.0

[PATCH v2 2/9] platform/x86: hp-bioscfg: fix heap OOB read in sk_store and kek_store

[Muhammad Bilal]

sk_store() and kek_store() strip a trailing newline from the sysfs
write before allocating the key buffer:

	length = count;
	if (buf[length - 1] == '\n')
		length--;
	bioscfg_drv.spm_data.signing_key = kmemdup(buf, length, GFP_KERNEL);

but then pass the original "count" (not "length") as the copy size to
hp_wmi_perform_query(), which memcpy()s that many bytes out of the
"length"-sized allocation, reading one byte past it whenever the write
ends in a newline, the normal case for a shell "echo" into sysfs.

KASAN confirms this directly:

  BUG: KASAN: slab-out-of-bounds in hp_wmi_perform_query+0x1e9/0x460 [hp_bioscfg]
  Read of size 28 at addr ffff88813c8e2b80 by task python3/16022
  ...
  sk_store+0xa7/0x240 [hp_bioscfg]
  kernfs_fop_write_iter+0x3e1/0x5d0
  ...
  The buggy address is located 0 bytes inside of
  allocated 27-byte region [ffff88813c8e2b80, ffff88813c8e2b9b)

Reproduced identically for kek_store, and at multiple write sizes
(28, 57, 201 bytes), each time reading exactly one byte past a
kmemdup() allocation one byte smaller than the write.

Fix by passing "length" instead of "count" to hp_wmi_perform_query()
in both functions.

Fixes: b2715aa2e135 ("platform/x86: hp-bioscfg: spmobj-attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
index 2b00a14792e9..4d94e48c1a4c 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
@@ -238,7 +238,7 @@ static ssize_t sk_store(struct kobject *kobj,
 	ret = hp_wmi_perform_query(HPWMI_SECUREPLATFORM_SET_SK,
 				   HPWMI_SECUREPLATFORM,
 				   (void *)bioscfg_drv.spm_data.signing_key,
-				   count, 0);
+				   length, 0);
 
 	if (!ret) {
 		bioscfg_drv.spm_data.mechanism = SIGNING_KEY;
@@ -274,7 +274,7 @@ static ssize_t kek_store(struct kobject *kobj,
 	ret = hp_wmi_perform_query(HPWMI_SECUREPLATFORM_SET_KEK,
 				   HPWMI_SECUREPLATFORM,
 				   (void *)bioscfg_drv.spm_data.endorsement_key,
-				   count, 0);
+				   length, 0);
 
 	if (!ret) {
 		bioscfg_drv.spm_data.mechanism = ENDORSEMENT_KEY;
-- 
2.55.0

[PATCH v2 3/9] platform/x86: hp-bioscfg: fix heap OOB read on empty password write

[Muhammad Bilal]

validate_password_input() computes length = strlen(buf) and then
checks buf[length - 1] to strip a trailing newline, without checking
that length is nonzero first. Writing an empty string (a bare '\n')
to current_password or new_password gives length == 0, and
buf[length - 1] reads buf[-1], one byte before the heap allocation
holding the copied input.

KASAN confirms this directly:

  BUG: KASAN: slab-out-of-bounds in store_password_instance.constprop.0+0x223/0x2a0 [hp_bioscfg]
  Read of size 1 at addr ffff88811bd8da9f by task sh/13740
  ...
  store_password_instance.constprop.0+0x223/0x2a0 [hp_bioscfg]
  current_password_store+0x14/0x20 [hp_bioscfg]
  ...
  The buggy address is located 23 bytes to the right of
  allocated 8-byte region [ffff88811bd8da80, ffff88811bd8da88)

Reproduced identically via new_password_store. Execution continues
past the bad read (the garbage byte only affects whether "length" is
decremented by one), so the write completes and returns success; this
is a pure information read past the buffer, not a crash, but it is
still an out-of-bounds access KASAN correctly flags.

Fix by only checking buf[length - 1] when length is nonzero.

Fixes: 8646a3b5ee3a ("platform/x86: hp-bioscfg: passwdobj-attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 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 4d79eb8056a5..86fa03a5ee9a 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -66,7 +66,7 @@ static int validate_password_input(int instance_id, const char *buf)
 	struct password_data *password_data = &bioscfg_drv.password_data[instance_id];
 
 	length = strlen(buf);
-	if (buf[length - 1] == '\n')
+	if (length > 0 && buf[length - 1] == '\n')
 		length--;
 
 	if (length > MAX_PASSWD_SIZE)
-- 
2.55.0

[PATCH v2 4/9] platform/x86: hp-bioscfg: fix 16-byte heap overflow for empty auth token

[Muhammad Bilal]

hp_calculate_security_buffer() special-cases an empty authentication
string and returns a fixed 4 bytes (sizeof(u16) * 2). But
hp_populate_security_buffer() does not special-case that same input:
for any authentication string that does not start with BEAM_PREFIX,
including the empty string, it always builds "UTF_PREFIX +
authentication" and converts the result to UTF-16, writing a 2-byte
length header plus 2 bytes per character of "<utf-16/>" (9 characters),
20 bytes total, regardless of how long "authentication" itself is.

The caller, hp_set_attribute(), sizes its kmalloc() buffer using
hp_calculate_security_buffer()'s return value, so for an empty
authentication token it allocates 4 bytes for the security area but
hp_populate_security_buffer() then writes 20 bytes into it, a 16-byte
heap buffer overflow.

The authentication token used here is the current admin/setup
password, which is an empty string by default until one is
configured. Any write to a writable BIOS attribute while no admin
password has been set reaches this path.

Fix by removing the special-case short return for an empty string in
hp_calculate_security_buffer() and letting the normal formula run,
which already accounts for the UTF_PREFIX correctly for the non-empty
case; for an empty string this naturally yields the same 20 bytes
that hp_populate_security_buffer() writes.

Fixes: b2715aa2e135 ("platform/x86: hp-bioscfg: spmobj-attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 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 4d94e48c1a4c..2d4a3720f80c 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
@@ -48,7 +48,7 @@ size_t hp_calculate_security_buffer(const char *authentication)
 
 	authlen = strlen(authentication);
 	if (!authlen)
-		return sizeof(u16) * 2;
+		return sizeof(u16) + strlen(UTF_PREFIX) * sizeof(u16);
 
 	size = sizeof(u16) + authlen * sizeof(u16);
 	if (!strstarts(authentication, BEAM_PREFIX))
-- 
2.55.0

Re: [PATCH v2 4/9] platform/x86: hp-bioscfg: fix 16-byte heap overflow for empty auth token

[Ilpo Järvinen]

On Wed, 12 Aug 2026, Muhammad Bilal wrote:

> hp_calculate_security_buffer() special-cases an empty authentication
> string and returns a fixed 4 bytes (sizeof(u16) * 2). But
> hp_populate_security_buffer() does not special-case that same input:
> for any authentication string that does not start with BEAM_PREFIX,
> including the empty string, it always builds "UTF_PREFIX +
> authentication" and converts the result to UTF-16, writing a 2-byte
> length header plus 2 bytes per character of "<utf-16/>" (9 characters),
> 20 bytes total, regardless of how long "authentication" itself is.
> 
> The caller, hp_set_attribute(), sizes its kmalloc() buffer using
> hp_calculate_security_buffer()'s return value, so for an empty
> authentication token it allocates 4 bytes for the security area but
> hp_populate_security_buffer() then writes 20 bytes into it, a 16-byte
> heap buffer overflow.
> 
> The authentication token used here is the current admin/setup
> password, which is an empty string by default until one is
> configured. Any write to a writable BIOS attribute while no admin
> password has been set reaches this path.
> 
> Fix by removing the special-case short return for an empty string in
> hp_calculate_security_buffer() and letting the normal formula run,
> which already accounts for the UTF_PREFIX correctly for the non-empty
> case; for an empty string this naturally yields the same 20 bytes
> that hp_populate_security_buffer() writes.
> 
> Fixes: b2715aa2e135 ("platform/x86: hp-bioscfg: spmobj-attributes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
>  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 4d94e48c1a4c..2d4a3720f80c 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
> @@ -48,7 +48,7 @@ size_t hp_calculate_security_buffer(const char *authentication)
>  
>  	authlen = strlen(authentication);
>  	if (!authlen)
> -		return sizeof(u16) * 2;
> +		return sizeof(u16) + strlen(UTF_PREFIX) * sizeof(u16);

There was an earlier attempt to fix this from Josh Snyder (added as 
recipient) by removing the entire return which looks better approach to me 
as it avoid code duplication for a special case:

  https://lore.kernel.org/platform-driver-x86/20260402-hp-bioscfg-overflow-v1-1-6985f8c9e67c@code406.com/

I'm not sure why there never was v2 of that patch from Josh with improved 
changelog text. Josh?

>  	size = sizeof(u16) + authlen * sizeof(u16);
>  	if (!strstarts(authentication, BEAM_PREFIX))
> 


-- 
 i.

[PATCH v2 5/9] platform/x86: hp-bioscfg: fix off-by-one heap OOB write in audit_log_entries_show

[Muhammad Bilal]

The per-iteration guard in audit_log_entries_show() is:

	if (ret < 0 || (LOG_ENTRY_SIZE * i) > PAGE_SIZE)
		break;
	...
	memcpy(buf, audit_log_buffer, LOG_ENTRY_SIZE);
	buf += LOG_ENTRY_SIZE;

At i == 256 (PAGE_SIZE / LOG_ENTRY_SIZE), LOG_ENTRY_SIZE * i equals
PAGE_SIZE exactly, which is not ">" PAGE_SIZE, so the loop does not
break and instead writes another LOG_ENTRY_SIZE (16) bytes starting at
offset 4096 of the page-sized sysfs output buffer, one entry past its
end.

This needs the BIOS to report more than 256 audit log entries, which
already exceeds this driver's own documented LOG_MAX_ENTRIES of 254,
so it requires a non-compliant or corrupted firmware value rather than
the roughly 85 million entries an unrelated integer-overflow read of
this code might suggest.

Fix by checking the bound against the offset the write is about to
reach, (i + 1), instead of the offset already written.

Fixes: 63e8f906e94e ("platform/x86: hp-bioscfg: surestart-attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
index b57e42f29282..6b63fdb84606 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
@@ -90,7 +90,7 @@ static ssize_t audit_log_entries_show(struct kobject *kobj,
 					   HPWMI_SURESTART,
 					   audit_log_buffer, 1, 128);
 
-		if (ret < 0 || (LOG_ENTRY_SIZE * i) > PAGE_SIZE) {
+		if (ret < 0 || (LOG_ENTRY_SIZE * (i + 1)) > PAGE_SIZE) {
 			/*
 			 * Encountered a failure while reading
 			 * individual logs. Only a partial list of
-- 
2.55.0

Re: [PATCH v2 5/9] platform/x86: hp-bioscfg: fix off-by-one heap OOB write in audit_log_entries_show

[Ilpo Järvinen]

On Wed, 12 Aug 2026, Muhammad Bilal wrote:

> The per-iteration guard in audit_log_entries_show() is:
> 
> 	if (ret < 0 || (LOG_ENTRY_SIZE * i) > PAGE_SIZE)
> 		break;
> 	...
> 	memcpy(buf, audit_log_buffer, LOG_ENTRY_SIZE);
> 	buf += LOG_ENTRY_SIZE;

Hi,

What about the preceeding check:

	if (count * LOG_ENTRY_SIZE > PAGE_SIZE)
                return -EIO;

Is the second check dead code or how can you get i large enough for it 
to ever be useful?

> At i == 256 (PAGE_SIZE / LOG_ENTRY_SIZE), LOG_ENTRY_SIZE * i equals
> PAGE_SIZE exactly, which is not ">" PAGE_SIZE, so the loop does not
> break and instead writes another LOG_ENTRY_SIZE (16) bytes starting at
> offset 4096 of the page-sized sysfs output buffer, one entry past its
> end.
> 
> This needs the BIOS to report more than 256 audit log entries, which
> already exceeds this driver's own documented LOG_MAX_ENTRIES of 254,
> so it requires a non-compliant or corrupted firmware value rather than
> the roughly 85 million entries an unrelated integer-overflow read of
> this code might suggest.
> 
> Fix by checking the bound against the offset the write is about to
> reach, (i + 1), instead of the offset already written.
> 
> Fixes: 63e8f906e94e ("platform/x86: hp-bioscfg: surestart-attributes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
> index b57e42f29282..6b63fdb84606 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
> @@ -90,7 +90,7 @@ static ssize_t audit_log_entries_show(struct kobject *kobj,
>  					   HPWMI_SURESTART,
>  					   audit_log_buffer, 1, 128);
>  
> -		if (ret < 0 || (LOG_ENTRY_SIZE * i) > PAGE_SIZE) {
> +		if (ret < 0 || (LOG_ENTRY_SIZE * (i + 1)) > PAGE_SIZE) {
>  			/*
>  			 * Encountered a failure while reading
>  			 * individual logs. Only a partial list of

Since this block uses break, the following else is unnecessary and should 
be removed in a separate patch.

-- 
 i.

[PATCH v2 6/9] platform/x86: hp-bioscfg: add missing bounds check in PSWD_ENCODINGS loop

[Muhammad Bilal]

The PREREQUISITES loop earlier in the same function checks
"elem + reqs" against password_obj_count before indexing the ACPI
package element array:

	if (elem + reqs >= password_obj_count) {
		pr_err("Error elem-objects package is too small\n");
		return -EINVAL;
	}

The PSWD_ENCODINGS loop performs the identical indexing pattern,
password_obj[elem + pos_values], with no equivalent check, causing an
out-of-bounds read of the package element array whenever
encodings_size is larger than the number of elements actually present.

Fix by adding the same bounds check, matching PREREQUISITES.

Fixes: 8646a3b5ee3a ("platform/x86: hp-bioscfg: passwdobj-attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index 86fa03a5ee9a..acb123985ede 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -351,6 +351,11 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
 		case PSWD_ENCODINGS:
 			size = min_t(u32, password_data->encodings_size, MAX_ENCODINGS_SIZE);
 			for (pos_values = 0; pos_values < size; pos_values++) {
+				if (elem + pos_values >= password_obj_count) {
+					pr_err("Error elem-objects package is too small\n");
+					return -EINVAL;
+				}
+
 				ret = hp_convert_hexstr_to_str(password_obj[elem + pos_values].string.pointer,
 							       password_obj[elem + pos_values].string.length,
 							       &str_value, &value_len);
-- 
2.55.0

[PATCH v2 7/9] platform/x86: hp-bioscfg: fix new_password_store overwriting current_password

[Muhammad Bilal]

current_password_store() and new_password_store() both call
store_password_instance() with is_current = true:

	static ssize_t new_password_store(...)
	{
		return store_password_instance(kobj, buf, count, true);
	}

so a write to new_password is routed to current_password instead, and
the new_password field is never written by either sysfs entry point.

Fix by passing false from new_password_store(), matching what the
is_current parameter is meant to select.

Fixes: 8646a3b5ee3a ("platform/x86: hp-bioscfg: passwdobj-attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 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 acb123985ede..6bd56d3f5bd0 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -123,7 +123,7 @@ static ssize_t new_password_store(struct kobject *kobj,
 				  struct kobj_attribute *attr,
 				  const char *buf, size_t count)
 {
-	return store_password_instance(kobj, buf, count, true);
+	return store_password_instance(kobj, buf, count, false);
 }
 
 static struct kobj_attribute password_new_password = __ATTR_WO(new_password);
-- 
2.55.0

[PATCH v2 8/9] platform/x86: hp-bioscfg: fix ORD_LIST_ELEMENTS never being parsed

[Muhammad Bilal]

The ACPI_TYPE_STRING case explicitly skips the string conversion for
elem == ORD_LIST_ELEMENTS:

	if (elem != PREREQUISITES && elem != ORD_LIST_ELEMENTS) {
		ret = hp_convert_hexstr_to_str(..., &str_value, &value_len);
		if (ret)
			continue;
	}

so by the time the ORD_LIST_ELEMENTS case in the eloc switch runs,
str_value is NULL (it was freed and reset to NULL at the end of the
previous iteration). That case then does:

	ret = hp_convert_hexstr_to_str(str_value, value_len, &tmpstr, &tmp_len);

hp_convert_hexstr_to_str() rejects a NULL input with -EINVAL, which
sends this function to exit_list, and exit_list unconditionally
returns 0. The net effect is that any ordered-list attribute with
elements present silently ends up with an empty elements list, with no
error surfaced anywhere.

Fix by converting the current element directly, order_obj[elem], the
same way the PREREQUISITES case already handles its own array
elements, instead of reusing the unrelated str_value/value_len left
over from earlier processing.

Fixes: 4b2672ec71a3 ("platform/x86: hp-bioscfg: order-list-attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
index f09489a085c8..704c69c18146 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -261,7 +261,9 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
 			 * Ordered list data is stored in hex and comma separated format
 			 * Convert the data and split it to show each element
 			 */
-			ret = hp_convert_hexstr_to_str(str_value, value_len, &tmpstr, &tmp_len);
+			ret = hp_convert_hexstr_to_str(order_obj[elem].string.pointer,
+						       order_obj[elem].string.length,
+						       &tmpstr, &tmp_len);
 			if (ret)
 				goto exit_list;
 
-- 
2.55.0

[PATCH v2 9/9] platform/x86: hp-bioscfg: advance elem past consumed array elements

[Muhammad Bilal]

The outer parsing loop in each attribute-type parser advances "elem"
(the index into the ACPI package element array) by exactly one per
iteration, but cases that consume multi-element arrays
(PREREQUISITES, ENUM_POSSIBLE_VALUES, PSWD_ENCODINGS) read "size"
consecutive elements without adjusting "elem" for the extra entries
consumed beyond the first. The next outer iteration then re-reads a
leftover element from the array just consumed instead of the next
real property, and the type check fails on that stale element,
aborting the parse with -EIO.

This produces exactly the failure visible in dmesg on the test
hardware, on every boot:

  Error expected type 2 for elem 13, but got type 1 instead
  hp_bioscfg: Returned error 0x3, "Invalid command value/Feature not
  supported"

Fix by advancing "elem" by (size - 1) after each array-consuming
loop, so the outer loop's own "elem++" lands on the correct next
element. "eloc" is intentionally left alone: it indexes the logical
property schema, not the physical element array, and each array case
is still exactly one logical property regardless of how many physical
elements it spans.

The defect is identical across all five attribute-type parsers
(enum, integer, string, ordered-list, password), which were
copy-pasted from the same template when the driver was introduced.

Fixes: 6b2770bfd6f9 ("platform/x86: hp-bioscfg: enum-attributes")
Fixes: 6f2c06d5a467 ("platform/x86: hp-bioscfg: int-attributes")
Fixes: e6c7b3e15559 ("platform/x86: hp-bioscfg: string-attributes")
Fixes: 4b2672ec71a3 ("platform/x86: hp-bioscfg: order-list-attributes")
Fixes: 8646a3b5ee3a ("platform/x86: hp-bioscfg: passwdobj-attributes")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c       | 4 ++++
 drivers/platform/x86/hp/hp-bioscfg/int-attributes.c        | 2 ++
 drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c | 2 ++
 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c  | 4 ++++
 drivers/platform/x86/hp/hp-bioscfg/string-attributes.c     | 2 ++
 5 files changed, 14 insertions(+)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
index af4d1920d488..43beb639051e 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
@@ -227,6 +227,8 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
 				kfree(str_value);
 				str_value = NULL;
 			}
+			if (size)
+				elem += size - 1;
 			break;
 
 		case SECURITY_LEVEL:
@@ -280,6 +282,8 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
 				kfree(str_value);
 				str_value = NULL;
 			}
+			if (size)
+				elem += (size < MAX_VALUES_SIZE ? size : MAX_VALUES_SIZE) - 1;
 			break;
 		default:
 			pr_warn("Invalid element: %d found in Enumeration attribute or data may be malformed\n", elem);
diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
index d96e160953e3..5373af71549a 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
@@ -243,6 +243,8 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
 				kfree(str_value);
 				str_value = NULL;
 			}
+			if (size)
+				elem += size - 1;
 			break;
 
 		case SECURITY_LEVEL:
diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
index 704c69c18146..6696255738ba 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -232,6 +232,8 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
 				kfree(str_value);
 				str_value = NULL;
 			}
+			if (size)
+				elem += size - 1;
 			break;
 
 		case SECURITY_LEVEL:
diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index 6bd56d3f5bd0..9b989ef756ea 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -321,6 +321,8 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
 				str_value = NULL;
 
 			}
+			if (size)
+				elem += size - 1;
 			break;
 		case SECURITY_LEVEL:
 			password_data->common.security_level = int_value;
@@ -367,6 +369,8 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
 				str_value = NULL;
 
 			}
+			if (size)
+				elem += size - 1;
 			break;
 		case PSWD_IS_SET:
 			password_data->is_enabled = int_value;
diff --git a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
index fe5a9a3a4ef1..5abec8995911 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
@@ -233,6 +233,8 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
 				kfree(str_value);
 				str_value = NULL;
 			}
+			if (size)
+				elem += size - 1;
 			break;
 
 		case SECURITY_LEVEL:
-- 
2.55.0

Re: [PATCH v2 0/9] platform/x86: hp-bioscfg: fix multiple memory safety bugs and parsing errors

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 3655 bytes --]

On Wed, 12 Aug 2026, Muhammad Bilal wrote:

> This series fixes several bugs in the hp-bioscfg driver:
> 
> 1. Memory safety issues (patches 1-6):
>    - Off-by-one NUL terminator write in hp_get_string_from_buffer()
>    - Heap OOB read in sk_store()/kek_store() when passing original
>      count instead of trimmed length to hp_wmi_perform_query()
>    - Heap OOB read on empty password write in validate_password_input()
>    - 16-byte heap overflow in hp_calculate_security_buffer() for empty
>      authentication tokens
>    - Off-by-one heap OOB write in audit_log_entries_show() when more
>      than 256 log entries are reported
>    - Missing bounds check in the PSWD_ENCODINGS parsing loop
> 
> 2. Logic bugs (patches 7-8):
>    - new_password_store() incorrectly passing is_current=true, causing
>      writes to land in current_password instead
>    - ORD_LIST_ELEMENTS case using a stale NULL str_value pointer
>      instead of converting the current ACPI element directly
> 
> 3. ACPI package element index tracking (patch 9):
>    - Five attribute-type parsers (enum, int, string, order-list,
>      passwd) all share the same defect: multi-element array cases
>      consume "size" consecutive elements but the outer loop only
>      advances by one, causing the next iteration to misread a
>      leftover array entry as the next property and abort with -EIO
> 
> Tested on HP EliteBook 840 G2 (BIOS M71 Ver. 01.31), kernel 7.2.0-rc5+,
> with CONFIG_KASAN_GENERIC=y and CONFIG_SLUB_DEBUG=y.
> 
> This series applies on top of:
>   commit ea4d8f8ba283 ("platform/x86: hp-bioscfg: fix
>   slab-out-of-bounds write in hp_convert_hexstr_to_str")
> 
> Changes in v2:
>   - Squash patches 9-13 from v1 into a single patch (now patch 9),
>     as the fix is identical across all five attribute-type parsers.
>     Requested by Ilpo Järvinen.
> 
> v1: https://lore.kernel.org/all/20260803143037.93105-1-meatuni001@gmail.com/
> 
> Muhammad Bilal (9):
>   platform/x86: hp-bioscfg: fix off-by-one write in
>     hp_get_string_from_buffer
>   platform/x86: hp-bioscfg: fix heap OOB read in sk_store and kek_store
>   platform/x86: hp-bioscfg: fix heap OOB read on empty password write
>   platform/x86: hp-bioscfg: fix 16-byte heap overflow for empty auth
>     token
>   platform/x86: hp-bioscfg: fix off-by-one heap OOB write in
>     audit_log_entries_show
>   platform/x86: hp-bioscfg: add missing bounds check in PSWD_ENCODINGS
>     loop
>   platform/x86: hp-bioscfg: fix new_password_store overwriting
>     current_password
>   platform/x86: hp-bioscfg: fix ORD_LIST_ELEMENTS never being parsed
>   platform/x86: hp-bioscfg: advance elem past consumed array elements

Hi,

Thanks for the update, I've applied patches 1-3 and 6-9 to the 
review-ilpo-next branch. For patch 4, the approach in Josh's patch seem 
better (IMO) and patch 5 seemed to be changing dead code (AFAICT).

(And once again, I really hate the amount of unnecessary copy paste in 
this driver.)

-- 
 i.

>  drivers/platform/x86/hp/hp-bioscfg/bioscfg.c        |  2 +-
>  .../platform/x86/hp/hp-bioscfg/enum-attributes.c    |  4 ++++
>  drivers/platform/x86/hp/hp-bioscfg/int-attributes.c |  2 ++
>  .../x86/hp/hp-bioscfg/order-list-attributes.c       |  6 +++++-
>  .../x86/hp/hp-bioscfg/passwdobj-attributes.c        | 13 +++++++++++--
>  .../platform/x86/hp/hp-bioscfg/spmobj-attributes.c  |  6 +++---
>  .../platform/x86/hp/hp-bioscfg/string-attributes.c  |  2 ++
>  .../x86/hp/hp-bioscfg/surestart-attributes.c        |  2 +-
>  8 files changed, 29 insertions(+), 8 deletions(-)
> 
> 

Re: [PATCH v2 0/9] platform/x86: hp-bioscfg: fix multiple memory safety bugs and parsing errors

[Muhammad Bilal]

Hi Ilpo,

Thank you for applying patches 1-3 and 6-9 to review-ilpo-next, and thanks
for the heads-up regarding Guangshuo Li's earlier patch for the password
encodings bounds check.

I have submitted v3 for the remaining two patches (patches 4 and 5) here:
https://lore.kernel.org/r/20260818191120.38556-1-meatuni001@gmail.com

Once these remaining fixes settle, I will look into refactoring and
consolidating the duplicate ACPI element parsing logic across the
attribute files as you requested.

Thanks,
Muhammad

On Tue, Aug 18, 2026 at 5:08 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Wed, 12 Aug 2026, Muhammad Bilal wrote:
>
> > This series fixes several bugs in the hp-bioscfg driver:
> >
> > 1. Memory safety issues (patches 1-6):
> >    - Off-by-one NUL terminator write in hp_get_string_from_buffer()
> >    - Heap OOB read in sk_store()/kek_store() when passing original
> >      count instead of trimmed length to hp_wmi_perform_query()
> >    - Heap OOB read on empty password write in validate_password_input()
> >    - 16-byte heap overflow in hp_calculate_security_buffer() for empty
> >      authentication tokens
> >    - Off-by-one heap OOB write in audit_log_entries_show() when more
> >      than 256 log entries are reported
> >    - Missing bounds check in the PSWD_ENCODINGS parsing loop
> >
> > 2. Logic bugs (patches 7-8):
> >    - new_password_store() incorrectly passing is_current=true, causing
> >      writes to land in current_password instead
> >    - ORD_LIST_ELEMENTS case using a stale NULL str_value pointer
> >      instead of converting the current ACPI element directly
> >
> > 3. ACPI package element index tracking (patch 9):
> >    - Five attribute-type parsers (enum, int, string, order-list,
> >      passwd) all share the same defect: multi-element array cases
> >      consume "size" consecutive elements but the outer loop only
> >      advances by one, causing the next iteration to misread a
> >      leftover array entry as the next property and abort with -EIO
> >
> > Tested on HP EliteBook 840 G2 (BIOS M71 Ver. 01.31), kernel 7.2.0-rc5+,
> > with CONFIG_KASAN_GENERIC=y and CONFIG_SLUB_DEBUG=y.
> >
> > This series applies on top of:
> >   commit ea4d8f8ba283 ("platform/x86: hp-bioscfg: fix
> >   slab-out-of-bounds write in hp_convert_hexstr_to_str")
> >
> > Changes in v2:
> >   - Squash patches 9-13 from v1 into a single patch (now patch 9),
> >     as the fix is identical across all five attribute-type parsers.
> >     Requested by Ilpo Järvinen.
> >
> > v1: https://lore.kernel.org/all/20260803143037.93105-1-meatuni001@gmail.com/
> >
> > Muhammad Bilal (9):
> >   platform/x86: hp-bioscfg: fix off-by-one write in
> >     hp_get_string_from_buffer
> >   platform/x86: hp-bioscfg: fix heap OOB read in sk_store and kek_store
> >   platform/x86: hp-bioscfg: fix heap OOB read on empty password write
> >   platform/x86: hp-bioscfg: fix 16-byte heap overflow for empty auth
> >     token
> >   platform/x86: hp-bioscfg: fix off-by-one heap OOB write in
> >     audit_log_entries_show
> >   platform/x86: hp-bioscfg: add missing bounds check in PSWD_ENCODINGS
> >     loop
> >   platform/x86: hp-bioscfg: fix new_password_store overwriting
> >     current_password
> >   platform/x86: hp-bioscfg: fix ORD_LIST_ELEMENTS never being parsed
> >   platform/x86: hp-bioscfg: advance elem past consumed array elements
>
> Hi,
>
> Thanks for the update, I've applied patches 1-3 and 6-9 to the
> review-ilpo-next branch. For patch 4, the approach in Josh's patch seem
> better (IMO) and patch 5 seemed to be changing dead code (AFAICT).
>
> (And once again, I really hate the amount of unnecessary copy paste in
> this driver.)
>
> --
>  i.
>
> >  drivers/platform/x86/hp/hp-bioscfg/bioscfg.c        |  2 +-
> >  .../platform/x86/hp/hp-bioscfg/enum-attributes.c    |  4 ++++
> >  drivers/platform/x86/hp/hp-bioscfg/int-attributes.c |  2 ++
> >  .../x86/hp/hp-bioscfg/order-list-attributes.c       |  6 +++++-
> >  .../x86/hp/hp-bioscfg/passwdobj-attributes.c        | 13 +++++++++++--
> >  .../platform/x86/hp/hp-bioscfg/spmobj-attributes.c  |  6 +++---
> >  .../platform/x86/hp/hp-bioscfg/string-attributes.c  |  2 ++
> >  .../x86/hp/hp-bioscfg/surestart-attributes.c        |  2 +-
> >  8 files changed, 29 insertions(+), 8 deletions(-)
> >
> >