Re: [PATCH v2 0/9] platform/x86: hp-bioscfg: fix multiple memory safety bugs and parsing errors

[Muhammad Bilal]

Hi Ilpo,

Thank you for applying patches 1-3 and 6-9 to review-ilpo-next, and thanks
for the heads-up regarding Guangshuo Li's earlier patch for the password
encodings bounds check.

I have submitted v3 for the remaining two patches (patches 4 and 5) here:
https://lore.kernel.org/r/20260818191120.38556-1-meatuni001@xxxxxxxxx

Once these remaining fixes settle, I will look into refactoring and
consolidating the duplicate ACPI element parsing logic across the
attribute files as you requested.

Thanks,
Muhammad

On Tue, Aug 18, 2026 at 5:08 PM Ilpo Järvinen
<ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote:
>
> On Wed, 12 Aug 2026, Muhammad Bilal wrote:
>
> > This series fixes several bugs in the hp-bioscfg driver:
> >
> > 1. Memory safety issues (patches 1-6):
> >    - Off-by-one NUL terminator write in hp_get_string_from_buffer()
> >    - Heap OOB read in sk_store()/kek_store() when passing original
> >      count instead of trimmed length to hp_wmi_perform_query()
> >    - Heap OOB read on empty password write in validate_password_input()
> >    - 16-byte heap overflow in hp_calculate_security_buffer() for empty
> >      authentication tokens
> >    - Off-by-one heap OOB write in audit_log_entries_show() when more
> >      than 256 log entries are reported
> >    - Missing bounds check in the PSWD_ENCODINGS parsing loop
> >
> > 2. Logic bugs (patches 7-8):
> >    - new_password_store() incorrectly passing is_current=true, causing
> >      writes to land in current_password instead
> >    - ORD_LIST_ELEMENTS case using a stale NULL str_value pointer
> >      instead of converting the current ACPI element directly
> >
> > 3. ACPI package element index tracking (patch 9):
> >    - Five attribute-type parsers (enum, int, string, order-list,
> >      passwd) all share the same defect: multi-element array cases
> >      consume "size" consecutive elements but the outer loop only
> >      advances by one, causing the next iteration to misread a
> >      leftover array entry as the next property and abort with -EIO
> >
> > Tested on HP EliteBook 840 G2 (BIOS M71 Ver. 01.31), kernel 7.2.0-rc5+,
> > with CONFIG_KASAN_GENERIC=y and CONFIG_SLUB_DEBUG=y.
> >
> > This series applies on top of:
> >   commit ea4d8f8ba283 ("platform/x86: hp-bioscfg: fix
> >   slab-out-of-bounds write in hp_convert_hexstr_to_str")
> >
> > Changes in v2:
> >   - Squash patches 9-13 from v1 into a single patch (now patch 9),
> >     as the fix is identical across all five attribute-type parsers.
> >     Requested by Ilpo Järvinen.
> >
> > v1: https://lore.kernel.org/all/20260803143037.93105-1-meatuni001@xxxxxxxxx/
> >
> > Muhammad Bilal (9):
> >   platform/x86: hp-bioscfg: fix off-by-one write in
> >     hp_get_string_from_buffer
> >   platform/x86: hp-bioscfg: fix heap OOB read in sk_store and kek_store
> >   platform/x86: hp-bioscfg: fix heap OOB read on empty password write
> >   platform/x86: hp-bioscfg: fix 16-byte heap overflow for empty auth
> >     token
> >   platform/x86: hp-bioscfg: fix off-by-one heap OOB write in
> >     audit_log_entries_show
> >   platform/x86: hp-bioscfg: add missing bounds check in PSWD_ENCODINGS
> >     loop
> >   platform/x86: hp-bioscfg: fix new_password_store overwriting
> >     current_password
> >   platform/x86: hp-bioscfg: fix ORD_LIST_ELEMENTS never being parsed
> >   platform/x86: hp-bioscfg: advance elem past consumed array elements
>
> Hi,
>
> Thanks for the update, I've applied patches 1-3 and 6-9 to the
> review-ilpo-next branch. For patch 4, the approach in Josh's patch seem
> better (IMO) and patch 5 seemed to be changing dead code (AFAICT).
>
> (And once again, I really hate the amount of unnecessary copy paste in
> this driver.)
>
> --
>  i.
>
> >  drivers/platform/x86/hp/hp-bioscfg/bioscfg.c        |  2 +-
> >  .../platform/x86/hp/hp-bioscfg/enum-attributes.c    |  4 ++++
> >  drivers/platform/x86/hp/hp-bioscfg/int-attributes.c |  2 ++
> >  .../x86/hp/hp-bioscfg/order-list-attributes.c       |  6 +++++-
> >  .../x86/hp/hp-bioscfg/passwdobj-attributes.c        | 13 +++++++++++--
> >  .../platform/x86/hp/hp-bioscfg/spmobj-attributes.c  |  6 +++---
> >  .../platform/x86/hp/hp-bioscfg/string-attributes.c  |  2 ++
> >  .../x86/hp/hp-bioscfg/surestart-attributes.c        |  2 +-
> >  8 files changed, 29 insertions(+), 8 deletions(-)
> >
> >