[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

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