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