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

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

[-- Attachment #1: Type: text/plain, Size: 3655 bytes --]

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