mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] platform/x86: hp-bioscfg: Fix out-of-bounds array access in ACPI package parsing
@ 2025-12-26 11:42 Junrui Luo
  2025-12-29 14:14 ` Ilpo Järvinen
  0 siblings, 1 reply; 2+ messages in thread
From: Junrui Luo @ 2025-12-26 11:42 UTC (permalink / raw)
  To: Jorge Lopez, Hans de Goede, Ilpo Järvinen, Thomas Weißschuh
  Cc: platform-driver-x86, linux-kernel, Yuhao Jiang, Junrui Luo

The hp_populate_*_elements_from_package() functions in the hp-bioscfg
driver contain out-of-bounds array access vulnerabilities.

These functions parse ACPI packages into internal data structures using
a for loop with index variable 'elem' that iterates through
enum_obj/integer_obj/order_obj/password_obj/string_obj arrays.

When processing multi-element fields like PREREQUISITES and
ENUM_POSSIBLE_VALUES, these functions read multiple consecutive array
elements using expressions like 'enum_obj[elem + reqs]' and
'enum_obj[elem + pos_values]' within nested loops.

The bug is that the bounds check only validated elem, but did not consider
the additional offset when accessing elem + reqs or elem + pos_values.

The fix changes the bounds check to validate the actual accessed index.

Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Reported-by: Junrui Luo <moonafterrain@outlook.com>
Fixes: e6c7b3e15559 ("platform/x86: hp-bioscfg: string-attributes")
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
Changes in v2:
- Add a more detailed description
- Link to v1: https://lore.kernel.org/all/SYBPR01MB78816A828BF586364CF7A4FFAFA6A@SYBPR01MB7881.ausprd01.prod.outlook.com/
---
 drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c       | 4 ++--
 drivers/platform/x86/hp/hp-bioscfg/int-attributes.c        | 2 +-
 drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c | 5 +++++
 drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c  | 5 +++++
 drivers/platform/x86/hp/hp-bioscfg/string-attributes.c     | 2 +-
 5 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
index c50ad5880503..f346aad8e9d8 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
@@ -207,7 +207,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
 		case PREREQUISITES:
 			size = min_t(u32, enum_data->common.prerequisites_size, MAX_PREREQUISITES_SIZE);
 			for (reqs = 0; reqs < size; reqs++) {
-				if (elem >= enum_obj_count) {
+				if (elem + reqs >= enum_obj_count) {
 					pr_err("Error enum-objects package is too small\n");
 					return -EINVAL;
 				}
@@ -255,7 +255,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
 
 			for (pos_values = 0; pos_values < size && pos_values < MAX_VALUES_SIZE;
 			     pos_values++) {
-				if (elem >= enum_obj_count) {
+				if (elem + pos_values >= enum_obj_count) {
 					pr_err("Error enum-objects package is too small\n");
 					return -EINVAL;
 				}
diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
index 6c7f4d5fa9cb..63b1fda2be4e 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
@@ -227,7 +227,7 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
 			size = min_t(u32, integer_data->common.prerequisites_size, MAX_PREREQUISITES_SIZE);
 
 			for (reqs = 0; reqs < size; reqs++) {
-				if (elem >= integer_obj_count) {
+				if (elem + reqs >= integer_obj_count) {
 					pr_err("Error elem-objects package is too small\n");
 					return -EINVAL;
 				}
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 c6e57bb9d8b7..6a31f47ce3f5 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
@@ -216,6 +216,11 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
 			size = min_t(u32, ordered_list_data->common.prerequisites_size,
 				     MAX_PREREQUISITES_SIZE);
 			for (reqs = 0; reqs < size; reqs++) {
+				if (elem + reqs >= order_obj_count) {
+					pr_err("Error elem-objects package is too small\n");
+					return -EINVAL;
+				}
+
 				ret = hp_convert_hexstr_to_str(order_obj[elem + reqs].string.pointer,
 							       order_obj[elem + reqs].string.length,
 							       &str_value, &value_len);
diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
index 187b372123ed..ec79d9d50377 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
@@ -303,6 +303,11 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
 				     MAX_PREREQUISITES_SIZE);
 
 			for (reqs = 0; reqs < size; reqs++) {
+				if (elem + reqs >= password_obj_count) {
+					pr_err("Error elem-objects package is too small\n");
+					return -EINVAL;
+				}
+
 				ret = hp_convert_hexstr_to_str(password_obj[elem + reqs].string.pointer,
 							       password_obj[elem + reqs].string.length,
 							       &str_value, &value_len);
diff --git a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
index 27758b779b2d..7b885d25650c 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
@@ -217,7 +217,7 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
 				     MAX_PREREQUISITES_SIZE);
 
 			for (reqs = 0; reqs < size; reqs++) {
-				if (elem >= string_obj_count) {
+				if (elem + reqs >= string_obj_count) {
 					pr_err("Error elem-objects package is too small\n");
 					return -EINVAL;
 				}

---
base-commit: 9448598b22c50c8a5bb77a9103e2d49f134c9578
change-id: 20251226-fixes-91414deb7fd9

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] platform/x86: hp-bioscfg: Fix out-of-bounds array access in ACPI package parsing
  2025-12-26 11:42 [PATCH v2] platform/x86: hp-bioscfg: Fix out-of-bounds array access in ACPI package parsing Junrui Luo
@ 2025-12-29 14:14 ` Ilpo Järvinen
  0 siblings, 0 replies; 2+ messages in thread
From: Ilpo Järvinen @ 2025-12-29 14:14 UTC (permalink / raw)
  To: Junrui Luo, Jorge Lopez, Jorge Lopez
  Cc: Hans de Goede, Thomas Weißschuh, platform-driver-x86, LKML,
	Yuhao Jiang

On Fri, 26 Dec 2025, Junrui Luo wrote:

> The hp_populate_*_elements_from_package() functions in the hp-bioscfg
> driver contain out-of-bounds array access vulnerabilities.
> 
> These functions parse ACPI packages into internal data structures using
> a for loop with index variable 'elem' that iterates through
> enum_obj/integer_obj/order_obj/password_obj/string_obj arrays.
> 
> When processing multi-element fields like PREREQUISITES and
> ENUM_POSSIBLE_VALUES, these functions read multiple consecutive array
> elements using expressions like 'enum_obj[elem + reqs]' and
> 'enum_obj[elem + pos_values]' within nested loops.
> 
> The bug is that the bounds check only validated elem, but did not consider
> the additional offset when accessing elem + reqs or elem + pos_values.
> 
> The fix changes the bounds check to validate the actual accessed index.
> 
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Reported-by: Junrui Luo <moonafterrain@outlook.com>
> Fixes: e6c7b3e15559 ("platform/x86: hp-bioscfg: string-attributes")
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>

Hi,

Thanks for the update, I've now applied this to the review-ilpo-fixes 
branch.

However, I still find multiple eyerising points in these copy-pasted 
blocks. This code was originally added half-baked with outstanding 
comments from me about the unreasonable use of copy-paste (IIRC, Hans 
didn't notice my outstanding comments from an earlier version):

- Why are there number of variations what is done when 
  hp_convert_hexstr_to_str() fails between the different types, some 
  return -EINVAL, some do continue, some break?!?
- Should these obj_count checks actually occur before the loop?
  (-EINVAL is returned, so why do part of the work at all?)

(IMO, this whole code under case PREREQUISITES: should have been put 
into a helper, perhaps a macro if function is not viable to avoid code 
variations such as those fixed by this patch. IIRC, there was other use 
of copy-paste as well.)

-- 
 i.

> ---
> Changes in v2:
> - Add a more detailed description
> - Link to v1: https://lore.kernel.org/all/SYBPR01MB78816A828BF586364CF7A4FFAFA6A@SYBPR01MB7881.ausprd01.prod.outlook.com/
> ---
>  drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c       | 4 ++--
>  drivers/platform/x86/hp/hp-bioscfg/int-attributes.c        | 2 +-
>  drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c | 5 +++++
>  drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c  | 5 +++++
>  drivers/platform/x86/hp/hp-bioscfg/string-attributes.c     | 2 +-
>  5 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
> index c50ad5880503..f346aad8e9d8 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/enum-attributes.c
> @@ -207,7 +207,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
>  		case PREREQUISITES:
>  			size = min_t(u32, enum_data->common.prerequisites_size, MAX_PREREQUISITES_SIZE);
>  			for (reqs = 0; reqs < size; reqs++) {
> -				if (elem >= enum_obj_count) {
> +				if (elem + reqs >= enum_obj_count) {
>  					pr_err("Error enum-objects package is too small\n");
>  					return -EINVAL;
>  				}
> @@ -255,7 +255,7 @@ static int hp_populate_enumeration_elements_from_package(union acpi_object *enum
>  
>  			for (pos_values = 0; pos_values < size && pos_values < MAX_VALUES_SIZE;
>  			     pos_values++) {
> -				if (elem >= enum_obj_count) {
> +				if (elem + pos_values >= enum_obj_count) {
>  					pr_err("Error enum-objects package is too small\n");
>  					return -EINVAL;
>  				}
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
> index 6c7f4d5fa9cb..63b1fda2be4e 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/int-attributes.c
> @@ -227,7 +227,7 @@ static int hp_populate_integer_elements_from_package(union acpi_object *integer_
>  			size = min_t(u32, integer_data->common.prerequisites_size, MAX_PREREQUISITES_SIZE);
>  
>  			for (reqs = 0; reqs < size; reqs++) {
> -				if (elem >= integer_obj_count) {
> +				if (elem + reqs >= integer_obj_count) {
>  					pr_err("Error elem-objects package is too small\n");
>  					return -EINVAL;
>  				}
> 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 c6e57bb9d8b7..6a31f47ce3f5 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> @@ -216,6 +216,11 @@ static int hp_populate_ordered_list_elements_from_package(union acpi_object *ord
>  			size = min_t(u32, ordered_list_data->common.prerequisites_size,
>  				     MAX_PREREQUISITES_SIZE);
>  			for (reqs = 0; reqs < size; reqs++) {
> +				if (elem + reqs >= order_obj_count) {
> +					pr_err("Error elem-objects package is too small\n");
> +					return -EINVAL;
> +				}
> +
>  				ret = hp_convert_hexstr_to_str(order_obj[elem + reqs].string.pointer,
>  							       order_obj[elem + reqs].string.length,
>  							       &str_value, &value_len);
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> index 187b372123ed..ec79d9d50377 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c
> @@ -303,6 +303,11 @@ static int hp_populate_password_elements_from_package(union acpi_object *passwor
>  				     MAX_PREREQUISITES_SIZE);
>  
>  			for (reqs = 0; reqs < size; reqs++) {
> +				if (elem + reqs >= password_obj_count) {
> +					pr_err("Error elem-objects package is too small\n");
> +					return -EINVAL;
> +				}
> +
>  				ret = hp_convert_hexstr_to_str(password_obj[elem + reqs].string.pointer,
>  							       password_obj[elem + reqs].string.length,
>  							       &str_value, &value_len);
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
> index 27758b779b2d..7b885d25650c 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/string-attributes.c
> @@ -217,7 +217,7 @@ static int hp_populate_string_elements_from_package(union acpi_object *string_ob
>  				     MAX_PREREQUISITES_SIZE);
>  
>  			for (reqs = 0; reqs < size; reqs++) {
> -				if (elem >= string_obj_count) {
> +				if (elem + reqs >= string_obj_count) {
>  					pr_err("Error elem-objects package is too small\n");
>  					return -EINVAL;
>  				}
> 
> ---
> base-commit: 9448598b22c50c8a5bb77a9103e2d49f134c9578
> change-id: 20251226-fixes-91414deb7fd9
> 
> Best regards,
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-12-29 14:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-26 11:42 [PATCH v2] platform/x86: hp-bioscfg: Fix out-of-bounds array access in ACPI package parsing Junrui Luo
2025-12-29 14:14 ` Ilpo Järvinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome