[PATCH v3 0/2] platform/x86: hp-bioscfg: fix empty auth token overflow and clean up audit log loop

[PATCH v3 0/2] platform/x86: hp-bioscfg: fix empty auth token overflow and clean up audit log loop

[Muhammad Bilal]

Hi Ilpo,

This is v3 for the remaining two patches (patches 4 and 5) from the v2
series [2], addressing your review feedback:

- Patch 1/2 (was patch 4 in v2): Remove the special-case "if (!authlen)"
  return entirely in hp_calculate_security_buffer(). The generic
  calculation already naturally yields 20 bytes for empty strings,
  eliminating duplicate logic and fixing the 16-byte heap overflow
  (reported earlier by Josh Snyder [3]).

- Patch 2/2 (was patch 5 in v2): Remove the dead inner bounds check in
  audit_log_entries_show() since "count * LOG_ENTRY_SIZE > PAGE_SIZE" is
  already checked prior to entering the loop, and clean up the redundant
  "else" block after "break".

Changes in v3:
  - Patch 1/2: Remove the "if (!authlen)" check completely instead of
    adjusting the formula, avoiding code duplication as suggested by
    Ilpo Järvinen.
  - Patch 2/2: Remove the dead loop check and the redundant else block
    instead of adjusting loop boundary math.

Changes in v2:
  - Squashed v1 patches 9-13 into a single patch (applied in
    review-ilpo-next).

Link: https://lore.kernel.org/r/20260803143037.93105-1-meatuni001@gmail.com [1]
Link: https://lore.kernel.org/r/20260812111829.172273-1-meatuni001@gmail.com [2]
Link: https://lore.kernel.org/r/20260402-hp-bioscfg-overflow-v1-1-6985f8c9e67c@code406.com [3]

Muhammad Bilal (2):
  platform/x86: hp-bioscfg: fix 16-byte heap overflow for empty auth
    token
  platform/x86: hp-bioscfg: remove dead bounds check in
    audit_log_entries_show

 drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c    | 4 ----
 drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c | 9 +++------
 2 files changed, 3 insertions(+), 10 deletions(-)

-- 
2.43.0

[PATCH v3 1/2] 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, causing 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 early return for an empty string in
hp_calculate_security_buffer(). The generic calculation that follows
already accounts for the UTF_PREFIX correctly, which naturally yields
the same 20 bytes that hp_populate_security_buffer() writes for an empty
string, avoiding duplicate logic for special cases.

Fixes: b2715aa2e135 ("platform/x86: hp-bioscfg: spmobj-attributes")
Reported-by: Josh Snyder <josh@code406.com>
Closes: https://lore.kernel.org/platform-driver-x86/20260402-hp-bioscfg-overflow-v1-1-6985f8c9e67c@code406.com/
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v3:
  - Remove the special-case return entirely instead of adjusting its
    formula, avoiding code duplication as suggested by Ilpo Järvinen.
  - Credit Josh Snyder who previously noted this approach.

Changes in v2:
  - None for this patch; resubmitted as part of the v2 series.

Link: https://lore.kernel.org/r/20260803143037.93105-1-meatuni001@gmail.com [v1]
Link: https://lore.kernel.org/r/20260812111829.172273-1-meatuni001@gmail.com [v2]
---
 drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
index 4d94e48c1a4c..136585141e6e 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
@@ -47,10 +47,6 @@ size_t hp_calculate_security_buffer(const char *authentication)
 	if (!authentication)
 		return sizeof(u16) * 2;
 
-	authlen = strlen(authentication);
-	if (!authlen)
-		return sizeof(u16) * 2;
-
 	authlen = strlen(authentication);
 	size = sizeof(u16) + authlen * sizeof(u16);
 	if (!strstarts(authentication, BEAM_PREFIX))
-- 
2.43.0

[PATCH v3 2/2] platform/x86: hp-bioscfg: remove dead bounds check in audit_log_entries_show

[Muhammad Bilal]

audit_log_entries_show() checks "count * LOG_ENTRY_SIZE > PAGE_SIZE"
prior to entering the loop. Because "count" is already bounded to fit
within PAGE_SIZE, the inner loop check "(LOG_ENTRY_SIZE * i) > PAGE_SIZE"
can never be reached and is dead code.

Remove the redundant inner bounds check and eliminate the unnecessary
"else" block after "break" to clean up the loop control flow.

Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v3:
  - Remove the dead inner loop check completely instead of adjusting
    the boundary arithmetic, as noted by Ilpo Järvinen.
  - Remove the redundant else block after break.

Changes in v2:
  - None for this patch; resubmitted as part of the v2 series.

Link: https://lore.kernel.org/r/20260803143037.93105-1-meatuni001@gmail.com [v1]
Link: https://lore.kernel.org/r/20260812111829.172273-1-meatuni001@gmail.com [v2]
---
 drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
index b57e42f29282..3b20757a3e74 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
@@ -93,15 +93,12 @@ 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) {
-			/*
-			 * Encountered a failure while reading
-			 * individual logs. Only a partial list of
-			 * audit log will be returned.
-			 */
+		/*
+		 * Encountered a failure while reading individual logs.
+		 * Only a partial list of audit log will be returned.
+		 */
+		if (ret < 0)
 			break;
-		} else {
-			memcpy(buf, audit_log_buffer, LOG_ENTRY_SIZE);
-			buf += LOG_ENTRY_SIZE;
-		}
+
+		memcpy(buf, audit_log_buffer, LOG_ENTRY_SIZE);
+		buf += LOG_ENTRY_SIZE;
 	}
 
 	return i * LOG_ENTRY_SIZE;
-- 
2.43.0

Re: [PATCH v3 2/2] platform/x86: hp-bioscfg: remove dead bounds check in audit_log_entries_show

[Ilpo Järvinen]

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

On Wed, 19 Aug 2026, Muhammad Bilal wrote:

> audit_log_entries_show() checks "count * LOG_ENTRY_SIZE > PAGE_SIZE"
> prior to entering the loop. Because "count" is already bounded to fit
> within PAGE_SIZE, the inner loop check "(LOG_ENTRY_SIZE * i) > PAGE_SIZE"
> can never be reached and is dead code.
> 
> Remove the redundant inner bounds check and eliminate the unnecessary
> "else" block after "break" to clean up the loop control flow.
> 
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
> Changes in v3:
>   - Remove the dead inner loop check completely instead of adjusting
>     the boundary arithmetic, as noted by Ilpo Järvinen.
>   - Remove the redundant else block after break.
> 
> Changes in v2:
>   - None for this patch; resubmitted as part of the v2 series.
> 
> Link: https://lore.kernel.org/r/20260803143037.93105-1-meatuni001@gmail.com [v1]
> Link: https://lore.kernel.org/r/20260812111829.172273-1-meatuni001@gmail.com [v2]
> ---
>  drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
> index b57e42f29282..3b20757a3e74 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
> @@ -93,15 +93,12 @@ 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) {
> -			/*
> -			 * Encountered a failure while reading
> -			 * individual logs. Only a partial list of
> -			 * audit log will be returned.
> -			 */
> +		/*
> +		 * Encountered a failure while reading individual logs.
> +		 * Only a partial list of audit log will be returned.
> +		 */
> +		if (ret < 0)
>  			break;
> -		} else {
> -			memcpy(buf, audit_log_buffer, LOG_ENTRY_SIZE);
> -			buf += LOG_ENTRY_SIZE;
> -		}
> +
> +		memcpy(buf, audit_log_buffer, LOG_ENTRY_SIZE);
> +		buf += LOG_ENTRY_SIZE;
>  	}
>  
>  	return i * LOG_ENTRY_SIZE;
> 

Patch 1 didn't apply without special trickery, this patch 2 doesn't even 
get that far:

checking file drivers/platform/x86/hp/hp-bioscfg/surestart-attributes.c
patch: **** malformed patch at line 65:         }

:-(

-- 
 i.

Re: [PATCH v3 0/2] platform/x86: hp-bioscfg: fix empty auth token overflow and clean up audit log loop

[Ilpo Järvinen]

On Wed, 19 Aug 2026 00:11:06 +0500, Muhammad Bilal wrote:

> This is v3 for the remaining two patches (patches 4 and 5) from the v2
> series [2], addressing your review feedback:
> 
> - Patch 1/2 (was patch 4 in v2): Remove the special-case "if (!authlen)"
>   return entirely in hp_calculate_security_buffer(). The generic
>   calculation already naturally yields 20 bytes for empty strings,
>   eliminating duplicate logic and fixing the 16-byte heap overflow
>   (reported earlier by Josh Snyder [3]).
> 
> [...]

Thank you for your contribution, it has been applied to my local
review-ilpo-next branch. Note it will show up in the public
platform-drivers-x86/review-ilpo-next branch only once I've pushed my
local branch there, which might take a while.

FYI [if applicable to your patch], as per Linus' policy change, also
fixes are mostly routed through for-next unless the fix is for a
commit introduced in the most recent cycle or is clearly a regression
fix.

The list of commits applied:
[1/2] platform/x86: hp-bioscfg: fix 16-byte heap overflow for empty auth token
      commit: 5714a846afa8d7b8363aa134d4acbd129f68d049
[2/2] platform/x86: hp-bioscfg: remove dead bounds check in audit_log_entries_show
      (no commit info)

--
 i.

Re: [PATCH v3 1/2] platform/x86: hp-bioscfg: fix 16-byte heap overflow for empty auth token

[Ilpo Järvinen]

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

On Wed, 19 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, causing 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 early return for an empty string in
> hp_calculate_security_buffer(). The generic calculation that follows
> already accounts for the UTF_PREFIX correctly, which naturally yields
> the same 20 bytes that hp_populate_security_buffer() writes for an empty
> string, avoiding duplicate logic for special cases.
> 
> Fixes: b2715aa2e135 ("platform/x86: hp-bioscfg: spmobj-attributes")
> Reported-by: Josh Snyder <josh@code406.com>
> Closes: https://lore.kernel.org/platform-driver-x86/20260402-hp-bioscfg-overflow-v1-1-6985f8c9e67c@code406.com/
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
> Changes in v3:
>   - Remove the special-case return entirely instead of adjusting its
>     formula, avoiding code duplication as suggested by Ilpo Järvinen.
>   - Credit Josh Snyder who previously noted this approach.
> 
> Changes in v2:
>   - None for this patch; resubmitted as part of the v2 series.
> 
> Link: https://lore.kernel.org/r/20260803143037.93105-1-meatuni001@gmail.com [v1]
> Link: https://lore.kernel.org/r/20260812111829.172273-1-meatuni001@gmail.com [v2]
> ---
>  drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
> index 4d94e48c1a4c..136585141e6e 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
> @@ -47,10 +47,6 @@ size_t hp_calculate_security_buffer(const char *authentication)
>  	if (!authentication)
>  		return sizeof(u16) * 2;
>  
> -	authlen = strlen(authentication);
> -	if (!authlen)
> -		return sizeof(u16) * 2;
> -
>  	authlen = strlen(authentication);
>  	size = sizeof(u16) + authlen * sizeof(u16);

authlen is not uninitialized, so only the early return should be dropped.

I've dropped this patch from the review-ilpo-next branch.

-- 
 i.

[PATCH v4] 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, causing 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 early return for an empty string in
hp_calculate_security_buffer(). The generic calculation that follows
already accounts for the UTF_PREFIX correctly, which naturally yields
the same 20 bytes that hp_populate_security_buffer() writes for an empty
string, avoiding duplicate logic for special cases.

Fixes: b2715aa2e135 ("platform/x86: hp-bioscfg: spmobj-attributes")
Reported-by: Josh Snyder <josh@code406.com>
Closes: https://lore.kernel.org/platform-driver-x86/20260402-hp-bioscfg-overflow-v1-1-6985f8c9e67c@code406.com/
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
Changes in v4:
  - Straight re-generation against current mainline: v3 was written
    against a tree state that had a duplicate "authlen = strlen(...)"
    assignment (one copy meant to be removed, one meant to survive),
    which current mainline does not have. That mismatch is why Ilpo
    needed "special trickery" to apply v3, and why the manually-applied
    result ended up with no authlen assignment at all (reported by
    kernel test robot). The v3 diff itself was fine, as Ilpo confirmed
    ("authlen is not uninitialized, so only the early return should be
    dropped"); this is that same one change, regenerated from scratch
    against the actual current tree so it applies cleanly without manual
    intervention.

Changes in v3:
  - Remove the special-case return entirely instead of adjusting its
    formula, avoiding code duplication as suggested by Ilpo Järvinen.
  - Credit Josh Snyder who previously noted this approach.

Changes in v2:
  - None for this patch; resubmitted as part of the v2 series.

Link: https://lore.kernel.org/r/20260803143037.93105-1-meatuni001@gmail.com [v1]
Link: https://lore.kernel.org/r/20260812111829.172273-1-meatuni001@gmail.com [v2]
Link: https://lore.kernel.org/r/20260818191120.38556-2-meatuni001@gmail.com [v3]
---
 drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
index 4d94e48..abf8ce2 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/spmobj-attributes.c
@@ -47,9 +47,6 @@ size_t hp_calculate_security_buffer(const char *authentication)
 		return sizeof(u16) * 2;
 
 	authlen = strlen(authentication);
-	if (!authlen)
-		return sizeof(u16) * 2;
-
 	size = sizeof(u16) + authlen * sizeof(u16);
 	if (!strstarts(authentication, BEAM_PREFIX))
 		size += strlen(UTF_PREFIX) * sizeof(u16);
-- 
2.43.0