All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Muhammad Bilal <meatuni001@gmail.com>, Josh Snyder <josh@code406.com>
Cc: platform-driver-x86@vger.kernel.org, jorge.lopez2@hp.com,
	 Hans de Goede <hansg@kernel.org>,
	linux@weissschuh.net,  LKML <linux-kernel@vger.kernel.org>,
	stable@vger.kernel.org
Subject: Re: [PATCH v2 4/9] platform/x86: hp-bioscfg: fix 16-byte heap overflow for empty auth token
Date: Tue, 18 Aug 2026 15:05:09 +0300 (EEST)	[thread overview]
Message-ID: <6d9508e0-a4f2-a97a-d5c2-09941b36fe3e@linux.intel.com> (raw)
In-Reply-To: <20260812111829.172273-5-meatuni001@gmail.com>

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: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
>  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/20260402-hp-bioscfg-overflow-v1-1-6985f8c9e67c@code406.com/

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.


  reply	other threads:[~2026-08-18 12:05 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-12 11:18 [PATCH v2 0/9] platform/x86: hp-bioscfg: fix multiple memory safety bugs and parsing errors Muhammad Bilal
2026-08-12 11:18 ` [PATCH v2 1/9] platform/x86: hp-bioscfg: fix off-by-one write in hp_get_string_from_buffer Muhammad Bilal
2026-08-12 11:18 ` [PATCH v2 2/9] platform/x86: hp-bioscfg: fix heap OOB read in sk_store and kek_store Muhammad Bilal
2026-08-12 11:18 ` [PATCH v2 3/9] platform/x86: hp-bioscfg: fix heap OOB read on empty password write Muhammad Bilal
2026-08-12 11:18 ` [PATCH v2 4/9] platform/x86: hp-bioscfg: fix 16-byte heap overflow for empty auth token Muhammad Bilal
2026-08-18 12:05   ` Ilpo Järvinen [this message]
2026-08-12 11:18 ` [PATCH v2 5/9] platform/x86: hp-bioscfg: fix off-by-one heap OOB write in audit_log_entries_show Muhammad Bilal
2026-08-18 11:52   ` Ilpo Järvinen
2026-08-12 11:18 ` [PATCH v2 6/9] platform/x86: hp-bioscfg: add missing bounds check in PSWD_ENCODINGS loop Muhammad Bilal
2026-08-12 11:18 ` [PATCH v2 7/9] platform/x86: hp-bioscfg: fix new_password_store overwriting current_password Muhammad Bilal
2026-08-12 11:18 ` [PATCH v2 8/9] platform/x86: hp-bioscfg: fix ORD_LIST_ELEMENTS never being parsed Muhammad Bilal
2026-08-12 11:18 ` [PATCH v2 9/9] platform/x86: hp-bioscfg: advance elem past consumed array elements Muhammad Bilal
2026-08-18 12:08 ` [PATCH v2 0/9] platform/x86: hp-bioscfg: fix multiple memory safety bugs and parsing errors Ilpo Järvinen
2026-08-18 19:16   ` Muhammad Bilal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6d9508e0-a4f2-a97a-d5c2-09941b36fe3e@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=hansg@kernel.org \
    --cc=jorge.lopez2@hp.com \
    --cc=josh@code406.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@weissschuh.net \
    --cc=meatuni001@gmail.com \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.