[PATCH v2 0/3] hp-bioscfg: fix attribute enumeration on older HP BIOS

[PATCH v2 0/3] hp-bioscfg: fix attribute enumeration on older HP BIOS

[Muhammad Bilal]

The hp_bioscfg driver fails to enumerate BIOS attributes on
HP EliteBook 840 G2 (and potentially other older HP models) because:

  1. hp_init_bios_package_attribute() hard-fails when a WMI ACPI package
     contains fewer elements than the per-type expected count (11 < 13),
     even though only the first 10 common elements are required to
     register an attribute.

  2. hp_populate_enumeration_elements_from_package() returns -EIO and
     discards the entire attribute when any single element has an
     unexpected ACPI object type - typically after a BIOS AML error
     returns malformed data.

Hardware affected:
  HP EliteBook 840 G2 (DMI: Hewlett-Packard HP EliteBook 840 G2/2216)
  BIOS: M71 Ver. 01.31 (02/24/2020)

How to reproduce:
  1. Boot a kernel with CONFIG_HP_BIOSCFG=m on an HP EliteBook 840 G2
  2. modprobe hp_bioscfg
  3. Observe dmesg:
       hp_bioscfg: ACPI-package does not have enough elements: 11 < 13
       Error expected type 2 for elem 13, but got type 1 instead

Changes since v1:
  Patch 1/3 is new. Relaxing the element-count gate in patch 2/3 (v1's
  patch 1/2) lets packages shorter than the per-type ELEM_CNT constant
  reach hp_populate_*_elements_from_package(). Those loops don't bound
  themselves against the real package size - each one re-derives a
  "count" by reading ->package.count off elements[0], which is always
  a string (NAME) object, so it's actually reading ->string.length
  through the union. That was harmless while the old hard min_elements
  gate guaranteed a full ELEM_CNT-sized package on every call, but once
  patch 2/3 allows a shorter package through, the fixed ELEM_CNT loop
  bound walks past the end of the real elements[] array - a heap
  out-of-bounds read, on the exact EliteBook 840 G2 hardware this
  series targets.

  Patch 1/3 fixes this by threading the real, already-validated
  obj->package.count down into every hp_populate_*_package_data()
  wrapper instead of letting each one guess at it, and bounds
  hp_populate_ordered_list_elements_from_package()'s main loop (which
  previously ignored the count entirely) the same way. It's a no-op
  for any package that already meets today's ELEM_CNT minimums, and
  patch 2/3 is only safe to apply on top of it.

  Patches 2/3 and 3/3 are otherwise unchanged from v1.

  Thanks to Mario for the v1 Reviewed-by, carried forward on 2/3 and
  3/3 since those are unmodified. Armin's point about migrating to the
  buffer-based WMI API for correct marshaling is well taken as the
  longer-term fix; this series is meant as a minimal, backportable fix
  for the immediate enumeration failure and the OOB read it would
  otherwise reintroduce, not a replacement for that migration.

Testing notes:
  Tested on HP EliteBook 840 G2 running Arch Linux kernel 7.0.13-arch1-1.
  After patches, hp_bioscfg loads successfully and enumerates available
  BIOS attributes. Attributes with shortened packages are partially
  populated and accessible via sysfs. No regressions on systems that
  return full ELEM_CNT-element packages (patch 1/3 only changes
  behavior once patch 2/3's relaxed gate can hand it a shorter one).

Relevant dmesg (before fix):
  [   11.xxx] hp_bioscfg: ACPI-package does not have enough elements:
              11 < 13
  [   11.xxx] ACPI BIOS Error (bug): AE_AML_BUFFER_LIMIT,
              Index (0x000000032) is beyond end of object (length 0x32)
  [   11.xxx] ACPI Error: Aborting method \_SB.WMID.WQBE
  [   11.xxx] Error expected type 2 for elem 13, got type 1
  [   11.xxx] hp_bioscfg: Returned error 0x3


Muhammad Bilal (3):
  platform/x86: hp-bioscfg: pass validated element count to package
    parsers
  platform/x86: hp-bioscfg: accept reduced ACPI packages from older HP
    BIOS
  platform/x86: hp-bioscfg: warn on element type mismatch instead of
    failing

 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c     | 16 +++++++++++++---
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.h     |  8 ++++++++
 .../platform/x86/hp/hp-bioscfg/enum-attributes.c | 10 ++++++----
 .../platform/x86/hp/hp-bioscfg/int-attributes.c  |  3 ++-
 .../x86/hp/hp-bioscfg/order-list-attributes.c    |  7 ++++---
 .../x86/hp/hp-bioscfg/passwdobj-attributes.c     |  5 +++--
 .../x86/hp/hp-bioscfg/string-attributes.c        |  3 ++-
 7 files changed, 38 insertions(+), 14 deletions(-)

-- 
2.55.0

[PATCH v2 1/3] platform/x86: hp-bioscfg: pass validated element count to package parsers

[Muhammad Bilal]

hp_init_bios_package_attribute() validates obj->package.count against
min_elements and then hands off elements = obj->package.elements to
one of the five per-type hp_populate_*_package_data() wrappers
(string, integer, enumeration, ordered list, password). None of these
wrappers receive that validated count. Instead each one re-derives it
locally:

  hp_populate_integer_elements_from_package(integer_obj,
                                            integer_obj->package.count,
                                            instance_id);

integer_obj here is elements, i.e. a pointer to elements[0] (the NAME
field, always ACPI_TYPE_STRING). Reading ->package.count off a string
object aliases ->string.length in the underlying union acpi_object, so
the "count" passed down is not the real package size at all.

For string, integer, enumeration and password attributes,
hp_populate_*_elements_from_package() bounds its iteration using the
corresponding per-type ELEM_CNT constant (STR_ELEM_CNT,
INT_ELEM_CNT, ENUM_ELEM_CNT and PSWD_ELEM_CNT). This relies on
hp_init_bios_package_attribute() rejecting packages with fewer than
ELEM_CNT elements before invoking the parsers.

Relaxing that check would allow shorter packages to reach these
functions, making the fixed loop bounds unsafe.

hp_populate_ordered_list_elements_from_package() doesn't even use the
count for its main loop bound - it iterates unconditionally up to
ORD_ELEM_CNT:

  for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT; elem++, eloc++)

which relies entirely on the same coincidence.

This is only safe as long as every caller is guaranteed to hand these
functions a package with at least ELEM_CNT real elements. A following
change relaxes that guarantee to allow shorter packages through, which
would turn this into a real out-of-bounds heap read of the
elements[] array once the real count drops below the fixed ELEM_CNT
loop bound.

Fix this at the source: thread the real, already-validated
obj->package.count down through each *_package_data() wrapper instead
of letting the per-type code guess at it, and use it to also bound
hp_populate_ordered_list_elements_from_package()'s main loop. This is a
no-op for any package that already meets the existing ELEM_CNT
minimums, and is a prerequisite for safely accepting shorter packages.

Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c               | 5 +++++
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.h               | 5 +++++
 drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c       | 3 ++-
 drivers/platform/x86/hp/hp-bioscfg/int-attributes.c        | 3 ++-
 drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c | 7 ++++---
 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c  | 5 +++--
 drivers/platform/x86/hp/hp-bioscfg/string-attributes.c     | 3 ++-
 7 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
index 27fd6cd215290..768330d291da8 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
@@ -731,26 +731,31 @@ static int hp_init_bios_package_attribute(enum hp_wmi_data_type attr_type,
 	switch (attr_type) {
 	case HPWMI_STRING_TYPE:
 		ret = hp_populate_string_package_data(elements,
+						      obj->package.count,
 						      instance_id,
 						      attr_name_kobj);
 		break;
 	case HPWMI_INTEGER_TYPE:
 		ret = hp_populate_integer_package_data(elements,
+						       obj->package.count,
 						       instance_id,
 						       attr_name_kobj);
 		break;
 	case HPWMI_ENUMERATION_TYPE:
 		ret = hp_populate_enumeration_package_data(elements,
+							   obj->package.count,
 							   instance_id,
 							   attr_name_kobj);
 		break;
 	case HPWMI_ORDERED_LIST_TYPE:
 		ret = hp_populate_ordered_list_package_data(elements,
+							    obj->package.count,
 							    instance_id,
 							    attr_name_kobj);
 		break;
 	case HPWMI_PASSWORD_TYPE:
 		ret = hp_populate_password_package_data(elements,
+							obj->package.count,
 							instance_id,
 							attr_name_kobj);
 		break;
diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
index f1eec0e4ba075..416d7e7aaaae3 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
@@ -401,6 +401,7 @@ int hp_populate_string_buffer_data(u8 *buffer_ptr, u32 *buffer_size,
 int hp_alloc_string_data(void);
 void hp_exit_string_attributes(void);
 int hp_populate_string_package_data(union acpi_object *str_obj,
+				    int str_obj_count,
 				    int instance_id,
 				    struct kobject *attr_name_kobj);
 
@@ -411,6 +412,7 @@ int hp_populate_integer_buffer_data(u8 *buffer_ptr, u32 *buffer_size,
 int hp_alloc_integer_data(void);
 void hp_exit_integer_attributes(void);
 int hp_populate_integer_package_data(union acpi_object *integer_obj,
+				     int integer_obj_count,
 				     int instance_id,
 				     struct kobject *attr_name_kobj);
 
@@ -421,6 +423,7 @@ int hp_populate_enumeration_buffer_data(u8 *buffer_ptr, u32 *buffer_size,
 int hp_alloc_enumeration_data(void);
 void hp_exit_enumeration_attributes(void);
 int hp_populate_enumeration_package_data(union acpi_object *enum_obj,
+					 int enum_obj_count,
 					 int instance_id,
 					 struct kobject *attr_name_kobj);
 
@@ -432,6 +435,7 @@ int hp_populate_ordered_list_buffer_data(u8 *buffer_ptr,
 int hp_alloc_ordered_list_data(void);
 void hp_exit_ordered_list_attributes(void);
 int hp_populate_ordered_list_package_data(union acpi_object *order_obj,
+					  int order_obj_count,
 					  int instance_id,
 					  struct kobject *attr_name_kobj);
 
@@ -440,6 +444,7 @@ int hp_populate_password_buffer_data(u8 *buffer_ptr, u32 *buffer_size,
 				     int instance_id,
 				     struct kobject *attr_name_kobj);
 int hp_populate_password_package_data(union acpi_object *password_obj,
+				      int password_obj_count,
 				      int instance_id,
 				      struct kobject *attr_name_kobj);
 int hp_alloc_password_data(void);
diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
index af4d1920d4880..3aa2c440e0528 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
@@ -304,6 +304,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
  * @attr_name_kobj: The parent kernel object
  */
 int hp_populate_enumeration_package_data(union acpi_object *enum_obj,
+					 int enum_obj_count,
 					 int instance_id,
 					 struct kobject *attr_name_kobj)
 {
@@ -312,7 +313,7 @@ int hp_populate_enumeration_package_data(union acpi_object *enum_obj,
 	enum_data->attr_name_kobj = attr_name_kobj;
 
 	hp_populate_enumeration_elements_from_package(enum_obj,
-						      enum_obj->package.count,
+						      enum_obj_count,
 						      instance_id);
 	hp_update_attribute_permissions(enum_data->common.is_readonly,
 					&enumeration_current_val);
diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
index d96e160953e39..107e4cf1efb8a 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
@@ -279,6 +279,7 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
  * @attr_name_kobj: The parent kernel object
  */
 int hp_populate_integer_package_data(union acpi_object *integer_obj,
+				     int integer_obj_count,
 				     int instance_id,
 				     struct kobject *attr_name_kobj)
 {
@@ -286,7 +287,7 @@ int hp_populate_integer_package_data(union acpi_object *integer_obj,
 
 	integer_data->attr_name_kobj = attr_name_kobj;
 	hp_populate_integer_elements_from_package(integer_obj,
-						  integer_obj->package.count,
+						  integer_obj_count,
 						  instance_id);
 	hp_update_attribute_permissions(integer_data->common.is_readonly,
 					&integer_current_val);
diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
index f09489a085c86..a50d074125268 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -145,7 +145,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
 	if (!order_obj)
 		return -EINVAL;
 
-	for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT; elem++, eloc++) {
+	for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT && elem < order_obj_count; elem++, eloc++) {
 
 		switch (order_obj[elem].type) {
 		case ACPI_TYPE_STRING:
@@ -301,7 +301,8 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
  * @instance_id: The instance to enumerate
  * @attr_name_kobj: The parent kernel object
  */
-int hp_populate_ordered_list_package_data(union acpi_object *order_obj, int instance_id,
+int hp_populate_ordered_list_package_data(union acpi_object *order_obj, int order_obj_count,
+					  int instance_id,
 					  struct kobject *attr_name_kobj)
 {
 	struct ordered_list_data *ordered_list_data = &bioscfg_drv.ordered_list_data[instance_id];
@@ -309,7 +310,7 @@ int hp_populate_ordered_list_package_data(union acpi_object *order_obj, int inst
 	ordered_list_data->attr_name_kobj = attr_name_kobj;
 
 	hp_populate_ordered_list_elements_from_package(order_obj,
-						       order_obj->package.count,
+						       order_obj_count,
 						       instance_id);
 	hp_update_attribute_permissions(ordered_list_data->common.is_readonly,
 					&ordered_list_current_val);
diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index 4d79eb8056a5d..89316d90454d2 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -388,7 +388,8 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
  * @instance_id: The instance to enumerate
  * @attr_name_kobj: The parent kernel object
  */
-int hp_populate_password_package_data(union acpi_object *password_obj, int instance_id,
+int hp_populate_password_package_data(union acpi_object *password_obj, int password_obj_count,
+				      int instance_id,
 				      struct kobject *attr_name_kobj)
 {
 	struct password_data *password_data = &bioscfg_drv.password_data[instance_id];
@@ -396,7 +397,7 @@ int hp_populate_password_package_data(union acpi_object *password_obj, int insta
 	password_data->attr_name_kobj = attr_name_kobj;
 
 	hp_populate_password_elements_from_package(password_obj,
-						   password_obj->package.count,
+						   password_obj_count,
 						   instance_id);
 
 	hp_friendly_user_name_update(password_data->common.path,
diff --git a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
index fe5a9a3a4ef17..da5e81f1d188f 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
@@ -267,6 +267,7 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
  * @attr_name_kobj: The parent kernel object
  */
 int hp_populate_string_package_data(union acpi_object *string_obj,
+				    int string_obj_count,
 				    int instance_id,
 				    struct kobject *attr_name_kobj)
 {
@@ -275,7 +276,7 @@ int hp_populate_string_package_data(union acpi_object *string_obj,
 	string_data->attr_name_kobj = attr_name_kobj;
 
 	hp_populate_string_elements_from_package(string_obj,
-						 string_obj->package.count,
+						 string_obj_count,
 						 instance_id);
 
 	hp_update_attribute_permissions(string_data->common.is_readonly,
-- 
2.55.0

Re: [PATCH v2 1/3] platform/x86: hp-bioscfg: pass validated element count to package parsers

[Ilpo Järvinen]

On Sat, 4 Jul 2026, Muhammad Bilal wrote:

> hp_init_bios_package_attribute() validates obj->package.count against
> min_elements and then hands off elements = obj->package.elements to

quoting elements = obj->package.elements statement in this context is 
confusing. I don't think it's usually necessary to copy code like this to 
changelog, here you can just state "elements" (which even happens to match 
the struct member's name even).

> one of the five per-type hp_populate_*_package_data() wrappers
> (string, integer, enumeration, ordered list, password). None of these
> wrappers receive that validated count. Instead each one re-derives it
> locally:
> 
>   hp_populate_integer_elements_from_package(integer_obj,
>                                             integer_obj->package.count,
>                                             instance_id);
> 
> integer_obj here is elements, i.e. a pointer to elements[0] (the NAME
> field, always ACPI_TYPE_STRING). Reading ->package.count off a string
> object aliases ->string.length in the underlying union acpi_object, so
> the "count" passed down is not the real package size at all.

Only at this point you're actually telling what the problem is.

My suggestion is to rewrite this so that you start by telling that wrong 
size (count) is read and passed on by these functions (sort of summary of 
the problem). Then explain what caused that.

> For string, integer, enumeration and password attributes,
> hp_populate_*_elements_from_package() bounds its iteration using the
> corresponding per-type ELEM_CNT constant (STR_ELEM_CNT,
> INT_ELEM_CNT, ENUM_ELEM_CNT and PSWD_ELEM_CNT). This relies on
> hp_init_bios_package_attribute() rejecting packages with fewer than
> ELEM_CNT elements before invoking the parsers.
> 
> Relaxing that check would allow shorter packages to reach these
> functions, making the fixed loop bounds unsafe.

??

You might be doing this in some later patch but then you should say 
that an upcoming patch is going to relax this check, otherwise it comes 
out of nowhere.

So I think you're trying to solve two cases here:

1) Passing wrong count.
2) Allowing count less than what those consts define (but this is only 
needed for some later patch?).

But it's so that fixing 1 ends up also solving 2? I think the changelog 
should mostly focus on 1 and then only state in the end that it also
prepares for an upcoming change that requires supporting 2.

> hp_populate_ordered_list_elements_from_package() doesn't even use the
> count for its main loop bound - it iterates unconditionally up to
> ORD_ELEM_CNT:
> 
>   for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT; elem++, eloc++)
> 
> which relies entirely on the same coincidence.
> 
> This is only safe as long as every caller is guaranteed to hand these
> functions a package with at least ELEM_CNT real elements. A following
> change relaxes that guarantee to allow shorter packages through, which
> would turn this into a real out-of-bounds heap read of the
> elements[] array once the real count drops below the fixed ELEM_CNT
> loop bound.
> 
> Fix this at the source: thread the real, already-validated
> obj->package.count down through each *_package_data() wrapper instead

"thread down" sounds odd to my (non-native) ear in this context.

each *_package_data() wrapper -> *_package_data() wrappers

> of letting the per-type code guess at it, and use it to also bound

"guess at it" sounds odd to my ear and is now even correct given your 
explanation of the problem.

> hp_populate_ordered_list_elements_from_package()'s main loop.

> This is a
> no-op for any package that already meets the existing ELEM_CNT
> minimums, and is a prerequisite for safely accepting shorter packages.
> 
> Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
> Cc: stable@vger.kernel.org
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-bioscfg/bioscfg.c               | 5 +++++
>  drivers/platform/x86/hp/hp-bioscfg/bioscfg.h               | 5 +++++
>  drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c       | 3 ++-
>  drivers/platform/x86/hp/hp-bioscfg/int-attributes.c        | 3 ++-
>  drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c | 7 ++++---
>  drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c  | 5 +++--
>  drivers/platform/x86/hp/hp-bioscfg/string-attributes.c     | 3 ++-
>  7 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> index 27fd6cd215290..768330d291da8 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> @@ -731,26 +731,31 @@ static int hp_init_bios_package_attribute(enum hp_wmi_data_type attr_type,
>  	switch (attr_type) {
>  	case HPWMI_STRING_TYPE:
>  		ret = hp_populate_string_package_data(elements,
> +						      obj->package.count,
>  						      instance_id,
>  						      attr_name_kobj);
>  		break;
>  	case HPWMI_INTEGER_TYPE:
>  		ret = hp_populate_integer_package_data(elements,
> +						       obj->package.count,
>  						       instance_id,
>  						       attr_name_kobj);
>  		break;
>  	case HPWMI_ENUMERATION_TYPE:
>  		ret = hp_populate_enumeration_package_data(elements,
> +							   obj->package.count,
>  							   instance_id,
>  							   attr_name_kobj);
>  		break;
>  	case HPWMI_ORDERED_LIST_TYPE:
>  		ret = hp_populate_ordered_list_package_data(elements,
> +							    obj->package.count,
>  							    instance_id,
>  							    attr_name_kobj);
>  		break;
>  	case HPWMI_PASSWORD_TYPE:
>  		ret = hp_populate_password_package_data(elements,
> +							obj->package.count,
>  							instance_id,
>  							attr_name_kobj);
>  		break;
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
> index f1eec0e4ba075..416d7e7aaaae3 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
> +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
> @@ -401,6 +401,7 @@ int hp_populate_string_buffer_data(u8 *buffer_ptr, u32 *buffer_size,
>  int hp_alloc_string_data(void);
>  void hp_exit_string_attributes(void);
>  int hp_populate_string_package_data(union acpi_object *str_obj,
> +				    int str_obj_count,
>  				    int instance_id,
>  				    struct kobject *attr_name_kobj);
>  
> @@ -411,6 +412,7 @@ int hp_populate_integer_buffer_data(u8 *buffer_ptr, u32 *buffer_size,
>  int hp_alloc_integer_data(void);
>  void hp_exit_integer_attributes(void);
>  int hp_populate_integer_package_data(union acpi_object *integer_obj,
> +				     int integer_obj_count,
>  				     int instance_id,
>  				     struct kobject *attr_name_kobj);
>  
> @@ -421,6 +423,7 @@ int hp_populate_enumeration_buffer_data(u8 *buffer_ptr, u32 *buffer_size,
>  int hp_alloc_enumeration_data(void);
>  void hp_exit_enumeration_attributes(void);
>  int hp_populate_enumeration_package_data(union acpi_object *enum_obj,
> +					 int enum_obj_count,
>  					 int instance_id,
>  					 struct kobject *attr_name_kobj);
>  
> @@ -432,6 +435,7 @@ int hp_populate_ordered_list_buffer_data(u8 *buffer_ptr,
>  int hp_alloc_ordered_list_data(void);
>  void hp_exit_ordered_list_attributes(void);
>  int hp_populate_ordered_list_package_data(union acpi_object *order_obj,
> +					  int order_obj_count,
>  					  int instance_id,
>  					  struct kobject *attr_name_kobj);
>  
> @@ -440,6 +444,7 @@ int hp_populate_password_buffer_data(u8 *buffer_ptr, u32 *buffer_size,
>  				     int instance_id,
>  				     struct kobject *attr_name_kobj);
>  int hp_populate_password_package_data(union acpi_object *password_obj,
> +				      int password_obj_count,
>  				      int instance_id,
>  				      struct kobject *attr_name_kobj);
>  int hp_alloc_password_data(void);
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
> index af4d1920d4880..3aa2c440e0528 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
> @@ -304,6 +304,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
>   * @attr_name_kobj: The parent kernel object
>   */
>  int hp_populate_enumeration_package_data(union acpi_object *enum_obj,
> +					 int enum_obj_count,
>  					 int instance_id,
>  					 struct kobject *attr_name_kobj)
>  {
> @@ -312,7 +313,7 @@ int hp_populate_enumeration_package_data(union acpi_object *enum_obj,
>  	enum_data->attr_name_kobj = attr_name_kobj;
>  
>  	hp_populate_enumeration_elements_from_package(enum_obj,
> -						      enum_obj->package.count,
> +						      enum_obj_count,
>  						      instance_id);
>  	hp_update_attribute_permissions(enum_data->common.is_readonly,
>  					&enumeration_current_val);
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
> index d96e160953e39..107e4cf1efb8a 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
> @@ -279,6 +279,7 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
>   * @attr_name_kobj: The parent kernel object
>   */
>  int hp_populate_integer_package_data(union acpi_object *integer_obj,
> +				     int integer_obj_count,
>  				     int instance_id,
>  				     struct kobject *attr_name_kobj)
>  {
> @@ -286,7 +287,7 @@ int hp_populate_integer_package_data(union acpi_object *integer_obj,
>  
>  	integer_data->attr_name_kobj = attr_name_kobj;
>  	hp_populate_integer_elements_from_package(integer_obj,
> -						  integer_obj->package.count,
> +						  integer_obj_count,
>  						  instance_id);
>  	hp_update_attribute_permissions(integer_data->common.is_readonly,
>  					&integer_current_val);
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> index f09489a085c86..a50d074125268 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> @@ -145,7 +145,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
>  	if (!order_obj)
>  		return -EINVAL;
>  
> -	for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT; elem++, eloc++) {
> +	for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT && elem < order_obj_count; elem++, eloc++) {

This looks like a separate fix belonging to own patch. It allows you also 
to write more focused changelog text for each patch.

>  
>  		switch (order_obj[elem].type) {
>  		case ACPI_TYPE_STRING:
> @@ -301,7 +301,8 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
>   * @instance_id: The instance to enumerate
>   * @attr_name_kobj: The parent kernel object
>   */
> -int hp_populate_ordered_list_package_data(union acpi_object *order_obj, int instance_id,
> +int hp_populate_ordered_list_package_data(union acpi_object *order_obj, int order_obj_count,
> +					  int instance_id,
>  					  struct kobject *attr_name_kobj)
>  {
>  	struct ordered_list_data *ordered_list_data = &bioscfg_drv.ordered_list_data[instance_id];
> @@ -309,7 +310,7 @@ int hp_populate_ordered_list_package_data(union acpi_object *order_obj, int inst
>  	ordered_list_data->attr_name_kobj = attr_name_kobj;
>  
>  	hp_populate_ordered_list_elements_from_package(order_obj,
> -						       order_obj->package.count,
> +						       order_obj_count,
>  						       instance_id);
>  	hp_update_attribute_permissions(ordered_list_data->common.is_readonly,
>  					&ordered_list_current_val);
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> index 4d79eb8056a5d..89316d90454d2 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> @@ -388,7 +388,8 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
>   * @instance_id: The instance to enumerate
>   * @attr_name_kobj: The parent kernel object
>   */
> -int hp_populate_password_package_data(union acpi_object *password_obj, int instance_id,
> +int hp_populate_password_package_data(union acpi_object *password_obj, int password_obj_count,
> +				      int instance_id,
>  				      struct kobject *attr_name_kobj)
>  {
>  	struct password_data *password_data = &bioscfg_drv.password_data[instance_id];
> @@ -396,7 +397,7 @@ int hp_populate_password_package_data(union acpi_object *password_obj, int insta
>  	password_data->attr_name_kobj = attr_name_kobj;
>  
>  	hp_populate_password_elements_from_package(password_obj,
> -						   password_obj->package.count,
> +						   password_obj_count,
>  						   instance_id);
>  
>  	hp_friendly_user_name_update(password_data->common.path,
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
> index fe5a9a3a4ef17..da5e81f1d188f 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
> @@ -267,6 +267,7 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
>   * @attr_name_kobj: The parent kernel object
>   */
>  int hp_populate_string_package_data(union acpi_object *string_obj,
> +				    int string_obj_count,
>  				    int instance_id,
>  				    struct kobject *attr_name_kobj)
>  {
> @@ -275,7 +276,7 @@ int hp_populate_string_package_data(union acpi_object *string_obj,
>  	string_data->attr_name_kobj = attr_name_kobj;
>  
>  	hp_populate_string_elements_from_package(string_obj,
> -						 string_obj->package.count,
> +						 string_obj_count,
>  						 instance_id);
>  
>  	hp_update_attribute_permissions(string_data->common.is_readonly,
> 

-- 
 i.

Re: [PATCH v2 1/3] platform/x86: hp-bioscfg: pass validated element count to package parsers

[Muhammad Bilal]

Hi Ilpo,

I've posted v3 of the series, which splits the prerequisite changes into
separate patches and updates the commit messages based on your review.

https://lore.kernel.org/all/20260707202111.35414-1-meatuni001@gmail.com/

Thanks for your review.

On Mon, Jul 6, 2026 at 10:57 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Sat, 4 Jul 2026, Muhammad Bilal wrote:
>
> > hp_init_bios_package_attribute() validates obj->package.count against
> > min_elements and then hands off elements = obj->package.elements to
>
> quoting elements = obj->package.elements statement in this context is
> confusing. I don't think it's usually necessary to copy code like this to
> changelog, here you can just state "elements" (which even happens to match
> the struct member's name even).
>
> > one of the five per-type hp_populate_*_package_data() wrappers
> > (string, integer, enumeration, ordered list, password). None of these
> > wrappers receive that validated count. Instead each one re-derives it
> > locally:
> >
> >   hp_populate_integer_elements_from_package(integer_obj,
> >                                             integer_obj->package.count,
> >                                             instance_id);
> >
> > integer_obj here is elements, i.e. a pointer to elements[0] (the NAME
> > field, always ACPI_TYPE_STRING). Reading ->package.count off a string
> > object aliases ->string.length in the underlying union acpi_object, so
> > the "count" passed down is not the real package size at all.
>
> Only at this point you're actually telling what the problem is.
>
> My suggestion is to rewrite this so that you start by telling that wrong
> size (count) is read and passed on by these functions (sort of summary of
> the problem). Then explain what caused that.
>
> > For string, integer, enumeration and password attributes,
> > hp_populate_*_elements_from_package() bounds its iteration using the
> > corresponding per-type ELEM_CNT constant (STR_ELEM_CNT,
> > INT_ELEM_CNT, ENUM_ELEM_CNT and PSWD_ELEM_CNT). This relies on
> > hp_init_bios_package_attribute() rejecting packages with fewer than
> > ELEM_CNT elements before invoking the parsers.
> >
> > Relaxing that check would allow shorter packages to reach these
> > functions, making the fixed loop bounds unsafe.
>
> ??
>
> You might be doing this in some later patch but then you should say
> that an upcoming patch is going to relax this check, otherwise it comes
> out of nowhere.
>
> So I think you're trying to solve two cases here:
>
> 1) Passing wrong count.
> 2) Allowing count less than what those consts define (but this is only
> needed for some later patch?).
>
> But it's so that fixing 1 ends up also solving 2? I think the changelog
> should mostly focus on 1 and then only state in the end that it also
> prepares for an upcoming change that requires supporting 2.
>
> > hp_populate_ordered_list_elements_from_package() doesn't even use the
> > count for its main loop bound - it iterates unconditionally up to
> > ORD_ELEM_CNT:
> >
> >   for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT; elem++, eloc++)
> >
> > which relies entirely on the same coincidence.
> >
> > This is only safe as long as every caller is guaranteed to hand these
> > functions a package with at least ELEM_CNT real elements. A following
> > change relaxes that guarantee to allow shorter packages through, which
> > would turn this into a real out-of-bounds heap read of the
> > elements[] array once the real count drops below the fixed ELEM_CNT
> > loop bound.
> >
> > Fix this at the source: thread the real, already-validated
> > obj->package.count down through each *_package_data() wrapper instead
>
> "thread down" sounds odd to my (non-native) ear in this context.
>
> each *_package_data() wrapper -> *_package_data() wrappers
>
> > of letting the per-type code guess at it, and use it to also bound
>
> "guess at it" sounds odd to my ear and is now even correct given your
> explanation of the problem.
>
> > hp_populate_ordered_list_elements_from_package()'s main loop.
>
> > This is a
> > no-op for any package that already meets the existing ELEM_CNT
> > minimums, and is a prerequisite for safely accepting shorter packages.
> >
> > Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> > ---
> >  drivers/platform/x86/hp/hp-bioscfg/bioscfg.c               | 5 +++++
> >  drivers/platform/x86/hp/hp-bioscfg/bioscfg.h               | 5 +++++
> >  drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c       | 3 ++-
> >  drivers/platform/x86/hp/hp-bioscfg/int-attributes.c        | 3 ++-
> >  drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c | 7 ++++---
> >  drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c  | 5 +++--
> >  drivers/platform/x86/hp/hp-bioscfg/string-attributes.c     | 3 ++-
> >  7 files changed, 23 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > index 27fd6cd215290..768330d291da8 100644
> > --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> > @@ -731,26 +731,31 @@ static int hp_init_bios_package_attribute(enum hp_wmi_data_type attr_type,
> >       switch (attr_type) {
> >       case HPWMI_STRING_TYPE:
> >               ret = hp_populate_string_package_data(elements,
> > +                                                   obj->package.count,
> >                                                     instance_id,
> >                                                     attr_name_kobj);
> >               break;
> >       case HPWMI_INTEGER_TYPE:
> >               ret = hp_populate_integer_package_data(elements,
> > +                                                    obj->package.count,
> >                                                      instance_id,
> >                                                      attr_name_kobj);
> >               break;
> >       case HPWMI_ENUMERATION_TYPE:
> >               ret = hp_populate_enumeration_package_data(elements,
> > +                                                        obj->package.count,
> >                                                          instance_id,
> >                                                          attr_name_kobj);
> >               break;
> >       case HPWMI_ORDERED_LIST_TYPE:
> >               ret = hp_populate_ordered_list_package_data(elements,
> > +                                                         obj->package.count,
> >                                                           instance_id,
> >                                                           attr_name_kobj);
> >               break;
> >       case HPWMI_PASSWORD_TYPE:
> >               ret = hp_populate_password_package_data(elements,
> > +                                                     obj->package.count,
> >                                                       instance_id,
> >                                                       attr_name_kobj);
> >               break;
> > diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
> > index f1eec0e4ba075..416d7e7aaaae3 100644
> > --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
> > +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
> > @@ -401,6 +401,7 @@ int hp_populate_string_buffer_data(u8 *buffer_ptr, u32 *buffer_size,
> >  int hp_alloc_string_data(void);
> >  void hp_exit_string_attributes(void);
> >  int hp_populate_string_package_data(union acpi_object *str_obj,
> > +                                 int str_obj_count,
> >                                   int instance_id,
> >                                   struct kobject *attr_name_kobj);
> >
> > @@ -411,6 +412,7 @@ int hp_populate_integer_buffer_data(u8 *buffer_ptr, u32 *buffer_size,
> >  int hp_alloc_integer_data(void);
> >  void hp_exit_integer_attributes(void);
> >  int hp_populate_integer_package_data(union acpi_object *integer_obj,
> > +                                  int integer_obj_count,
> >                                    int instance_id,
> >                                    struct kobject *attr_name_kobj);
> >
> > @@ -421,6 +423,7 @@ int hp_populate_enumeration_buffer_data(u8 *buffer_ptr, u32 *buffer_size,
> >  int hp_alloc_enumeration_data(void);
> >  void hp_exit_enumeration_attributes(void);
> >  int hp_populate_enumeration_package_data(union acpi_object *enum_obj,
> > +                                      int enum_obj_count,
> >                                        int instance_id,
> >                                        struct kobject *attr_name_kobj);
> >
> > @@ -432,6 +435,7 @@ int hp_populate_ordered_list_buffer_data(u8 *buffer_ptr,
> >  int hp_alloc_ordered_list_data(void);
> >  void hp_exit_ordered_list_attributes(void);
> >  int hp_populate_ordered_list_package_data(union acpi_object *order_obj,
> > +                                       int order_obj_count,
> >                                         int instance_id,
> >                                         struct kobject *attr_name_kobj);
> >
> > @@ -440,6 +444,7 @@ int hp_populate_password_buffer_data(u8 *buffer_ptr, u32 *buffer_size,
> >                                    int instance_id,
> >                                    struct kobject *attr_name_kobj);
> >  int hp_populate_password_package_data(union acpi_object *password_obj,
> > +                                   int password_obj_count,
> >                                     int instance_id,
> >                                     struct kobject *attr_name_kobj);
> >  int hp_alloc_password_data(void);
> > diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
> > index af4d1920d4880..3aa2c440e0528 100644
> > --- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
> > +++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
> > @@ -304,6 +304,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
> >   * @attr_name_kobj: The parent kernel object
> >   */
> >  int hp_populate_enumeration_package_data(union acpi_object *enum_obj,
> > +                                      int enum_obj_count,
> >                                        int instance_id,
> >                                        struct kobject *attr_name_kobj)
> >  {
> > @@ -312,7 +313,7 @@ int hp_populate_enumeration_package_data(union acpi_object *enum_obj,
> >       enum_data->attr_name_kobj = attr_name_kobj;
> >
> >       hp_populate_enumeration_elements_from_package(enum_obj,
> > -                                                   enum_obj->package.count,
> > +                                                   enum_obj_count,
> >                                                     instance_id);
> >       hp_update_attribute_permissions(enum_data->common.is_readonly,
> >                                       &enumeration_current_val);
> > diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
> > index d96e160953e39..107e4cf1efb8a 100644
> > --- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
> > +++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
> > @@ -279,6 +279,7 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
> >   * @attr_name_kobj: The parent kernel object
> >   */
> >  int hp_populate_integer_package_data(union acpi_object *integer_obj,
> > +                                  int integer_obj_count,
> >                                    int instance_id,
> >                                    struct kobject *attr_name_kobj)
> >  {
> > @@ -286,7 +287,7 @@ int hp_populate_integer_package_data(union acpi_object *integer_obj,
> >
> >       integer_data->attr_name_kobj = attr_name_kobj;
> >       hp_populate_integer_elements_from_package(integer_obj,
> > -                                               integer_obj->package.count,
> > +                                               integer_obj_count,
> >                                                 instance_id);
> >       hp_update_attribute_permissions(integer_data->common.is_readonly,
> >                                       &integer_current_val);
> > diff --git a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> > index f09489a085c86..a50d074125268 100644
> > --- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> > +++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> > @@ -145,7 +145,7 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
> >       if (!order_obj)
> >               return -EINVAL;
> >
> > -     for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT; elem++, eloc++) {
> > +     for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT && elem < order_obj_count; elem++, eloc++) {
>
> This looks like a separate fix belonging to own patch. It allows you also
> to write more focused changelog text for each patch.
>
> >
> >               switch (order_obj[elem].type) {
> >               case ACPI_TYPE_STRING:
> > @@ -301,7 +301,8 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
> >   * @instance_id: The instance to enumerate
> >   * @attr_name_kobj: The parent kernel object
> >   */
> > -int hp_populate_ordered_list_package_data(union acpi_object *order_obj, int instance_id,
> > +int hp_populate_ordered_list_package_data(union acpi_object *order_obj, int order_obj_count,
> > +                                       int instance_id,
> >                                         struct kobject *attr_name_kobj)
> >  {
> >       struct ordered_list_data *ordered_list_data = &bioscfg_drv.ordered_list_data[instance_id];
> > @@ -309,7 +310,7 @@ int hp_populate_ordered_list_package_data(union acpi_object *order_obj, int inst
> >       ordered_list_data->attr_name_kobj = attr_name_kobj;
> >
> >       hp_populate_ordered_list_elements_from_package(order_obj,
> > -                                                    order_obj->package.count,
> > +                                                    order_obj_count,
> >                                                      instance_id);
> >       hp_update_attribute_permissions(ordered_list_data->common.is_readonly,
> >                                       &ordered_list_current_val);
> > diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> > index 4d79eb8056a5d..89316d90454d2 100644
> > --- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> > +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> > @@ -388,7 +388,8 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
> >   * @instance_id: The instance to enumerate
> >   * @attr_name_kobj: The parent kernel object
> >   */
> > -int hp_populate_password_package_data(union acpi_object *password_obj, int instance_id,
> > +int hp_populate_password_package_data(union acpi_object *password_obj, int password_obj_count,
> > +                                   int instance_id,
> >                                     struct kobject *attr_name_kobj)
> >  {
> >       struct password_data *password_data = &bioscfg_drv.password_data[instance_id];
> > @@ -396,7 +397,7 @@ int hp_populate_password_package_data(union acpi_object *password_obj, int insta
> >       password_data->attr_name_kobj = attr_name_kobj;
> >
> >       hp_populate_password_elements_from_package(password_obj,
> > -                                                password_obj->package.count,
> > +                                                password_obj_count,
> >                                                  instance_id);
> >
> >       hp_friendly_user_name_update(password_data->common.path,
> > diff --git a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
> > index fe5a9a3a4ef17..da5e81f1d188f 100644
> > --- a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
> > +++ b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
> > @@ -267,6 +267,7 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
> >   * @attr_name_kobj: The parent kernel object
> >   */
> >  int hp_populate_string_package_data(union acpi_object *string_obj,
> > +                                 int string_obj_count,
> >                                   int instance_id,
> >                                   struct kobject *attr_name_kobj)
> >  {
> > @@ -275,7 +276,7 @@ int hp_populate_string_package_data(union acpi_object *string_obj,
> >       string_data->attr_name_kobj = attr_name_kobj;
> >
> >       hp_populate_string_elements_from_package(string_obj,
> > -                                              string_obj->package.count,
> > +                                              string_obj_count,
> >                                                instance_id);
> >
> >       hp_update_attribute_permissions(string_data->common.is_readonly,
> >
>
> --
>  i.
>

[PATCH v2 2/3] platform/x86: hp-bioscfg: accept reduced ACPI packages from older HP BIOS

[Muhammad Bilal]

hp_init_bios_package_attribute() hard-fails when a WMI ACPI package
contains fewer elements than the type-specific expected count (e.g. 11
elements instead of 13 for INTEGER or ENUMERATION attributes). This
causes the entire hp_bioscfg driver to skip attribute enumeration on
older HP hardware whose BIOS returns shortened packages when optional
fields like prerequisites or possible values are absent.

Observed on HP EliteBook 840 G2 (BIOS M71 Ver. 01.31):

  hp_bioscfg: ACPI-package does not have enough elements: 11 < 13

The element layout has two tiers:
  - Elements 0-9 (SECURITY_LEVEL+1 = 10): common to all attribute types
  - Elements 10-N: type-specific (bounds, values, encodings, ...)

The per-type populate functions (hp_populate_*_elements_from_package)
already handle sparse packages correctly via their own elem < count
loop guards and inner-loop bounds checks. The only unsafe case is when
we lack even the common elements needed to register the attribute.

Fix by introducing COMMON_ELEM_CNT to mark the hard minimum (10), and
splitting the check into two tiers:
  - Fewer than COMMON_ELEM_CNT elements: hard fail, can't proceed.
  - Fewer than expected type-specific elements: warn, but let the
    populate function parse what is available.

Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 11 ++++++++---
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.h |  3 +++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
index 768330d291da8..78019644ec358 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
@@ -661,12 +661,17 @@ static int hp_init_bios_package_attribute(enum hp_wmi_data_type attr_type,
 	int ret = 0;
 
 	/* Take action appropriate to each ACPI TYPE */
-	if (obj->package.count < min_elements) {
-		pr_err("ACPI-package does not have enough elements: %d < %d\n",
-		       obj->package.count, min_elements);
+	if (obj->package.count < COMMON_ELEM_CNT) {
+		pr_err("ACPI-package is missing common elements: %d < %d\n",
+		       obj->package.count, COMMON_ELEM_CNT);
 		goto pack_attr_exit;
 	}
 
+	if (obj->package.count < min_elements) {
+		pr_warn("ACPI-package has fewer elements than expected: %d < %d, parsing available elements\n",
+			obj->package.count, min_elements);
+	}
+
 	elements = obj->package.elements;
 
 	/* sanity checking */
diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
index 416d7e7aaaae3..ac57d6eab4c35 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.h
@@ -279,6 +279,9 @@ enum hp_wmi_data_elements {
 	PSWD_ENCODINGS = 13,
 	PSWD_IS_SET = 14,
 	PSWD_ELEM_CNT = 15,
+
+	/* Minimum elements shared by all attribute types (NAME..SECURITY_LEVEL) */
+	COMMON_ELEM_CNT = SECURITY_LEVEL + 1,
 };
 
 #define GET_INSTANCE_ID(type)						\
-- 
2.55.0

[PATCH v2 3/3] platform/x86: hp-bioscfg: warn on element type mismatch instead of failing

[Muhammad Bilal]

hp_populate_enumeration_elements_from_package() returns -EIO and aborts
enumeration of the entire attribute when any single element has an
unexpected ACPI type. This is observed on HP EliteBook 840 G2 when the
BIOS returns malformed ACPI data following a failed WMI query:

  ACPI BIOS Error (bug): AE_AML_BUFFER_LIMIT, Index (0x000000032)
    is beyond end of object (length 0x32)
  ACPI Error: Aborting method \_SB.WMID.WQBE due to previous error
  Error expected type 2 for elem 13, but got type 1 instead
  hp_bioscfg: Returned error 0x3,
    "Invalid command value/Feature not supported"

A type mismatch on one element does not necessarily corrupt the attribute
being built, especially for non-critical type-specific elements such as
possible values or bounds. Failing fatally here discards attributes that
could otherwise be partially useful.

Change the type mismatch handling from a fatal pr_err + return -EIO to
a pr_warn + continue, freeing the accumulated string value and skipping
the affected element. The attribute is still registered with whatever
valid elements the BIOS did supply.

Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
index 3aa2c440e0528..b834303e5bc79 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
@@ -163,10 +163,11 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
 
 		/* Check that both expected and read object type match */
 		if (expected_enum_types[eloc] != enum_obj[elem].type) {
-			pr_err("Error expected type %d for elem %d, but got type %d instead\n",
-			       expected_enum_types[eloc], elem, enum_obj[elem].type);
+			pr_warn("Unexpected element type at elem %d: expected %d, got %d, skipping\n",
+				elem, expected_enum_types[eloc], enum_obj[elem].type);
 			kfree(str_value);
-			return -EIO;
+			str_value = NULL;
+			continue;
 		}
 
 		/* Assign appropriate element value to corresponding field */
-- 
2.55.0