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

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: [email protected]
> Signed-off-by: Muhammad Bilal <[email protected]>
> ---
>  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/[email protected]/

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.