Re: [PATCH] platform/x86: hp-bioscfg: fix heap buffer overflow in security buffer

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Thu, 2 Apr 2026, Josh Snyder wrote:

> hp_calculate_security_buffer() returns sizeof(u16) * 2 (4 bytes) for
> empty strings via an early return. However, hp_populate_security_buffer()
> always prepends UTF_PREFIX ("<utf-16/>") to non-BEAM authentication
> strings, including empty ones, before converting to UTF-16. This results
> in 20 bytes being written into a 4-byte region of the heap buffer
> allocated in hp_set_attribute().
> 
> The 16-byte overrun corrupts adjacent heap memory. In practice, the
> firmware's own bounds checking rejects the undersized buffer before
> acting on it (returning error 0x04), which masked the overflow.
> 
> Fix by removing the early return for empty strings. The calculation at
> the end of the function already accounts for UTF_PREFIX correctly when
> authlen is zero: sizeof(u16) + 0 + strlen("<utf-16/>") * sizeof(u16)
> = 20 bytes, matching what hp_populate_security_buffer() writes. The
> NULL check is preserved since hp_populate_security_buffer() would

What NULL check this refers to??? For which of the inputs?

> dereference NULL via strstarts().

This entire description is confusing and seems to also contradict with 
itself in multiple places.

> Fixes: b2715aa2e1352 ("platform/x86: hp-bioscfg: spmobj-attributes")
> Assisted-by: Claude:claude-opus-4-6
> Signed-off-by: Josh Snyder <josh@code406.com>
> ---
>  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 2b00a14792e92..93e0a077e4240 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);
> 
> ---
> base-commit: cc13002a9f984d37906e9476f3e532a8cdd126f5
> change-id: 20260402-hp-bioscfg-overflow-d58dbe53ecf5
> 
> Best regards,
> --  
> Josh
> 

-- 
 i.