Re: [PATCH v2 0/9] platform/x86: hp-bioscfg: fix multiple memory safety bugs and parsing errors
Muhammad Bilal <[email protected]>
| Newsgroups | org.kernel.vger.platform-driver-x86,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <CADqcGBnCt-Rkxa3ms7g-nbe2+Js9fyzSH52KQPNZBvoeTiaRcg@mail.gmail.com> |
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/[email protected] 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 <[email protected]> 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/[email protected]/ > > > > 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(-) > > > >