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

Muhammad Bilal posted 9 patches 1 month, 2 weeks ago
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(-)
[PATCH v2 0/9] platform/x86: hp-bioscfg: fix multiple memory safety bugs and parsing errors
Posted by Muhammad Bilal 1 month, 2 weeks ago
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@gmail.com/

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

 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(-)

-- 
2.55.0

Re: [PATCH v2 0/9] platform/x86: hp-bioscfg: fix multiple memory safety bugs and parsing errors
Posted by Ilpo Järvinen 1 month, 1 week ago
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@gmail.com/
> 
> 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(-)
> 
> 
Re: [PATCH v2 0/9] platform/x86: hp-bioscfg: fix multiple memory safety bugs and parsing errors
Posted by Muhammad Bilal 1 month, 1 week ago
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@gmail.com

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@linux.intel.com> 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@gmail.com/
> >
> > 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(-)
> >
> >