From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Muhammad Bilal <meatuni001@gmail.com>
Cc: hansg@kernel.org, ilpo.jarvinen@linux.intel.com,
jorge.lopez2@hp.com, linux@weissschuh.net,
platform-driver-x86@vger.kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org,
Mario Limonciello <superm1@kernel.org>,
Armin Wolf <W_Armin@gmx.de>
Subject: Re: [PATCH v4 1/4] platform/x86: hp-bioscfg: pass validated element count to package parsers
Date: Thu, 9 Jul 2026 16:26:59 +0300 (EEST) [thread overview]
Message-ID: <86309036-a9b3-fa41-0abd-f4eaad6b53fa@linux.intel.com> (raw)
In-Reply-To: <20260708154846.12356-2-meatuni001@gmail.com>
On Wed, 8 Jul 2026, Muhammad Bilal wrote:
> The per-type package parsers are handed the wrong element count.
>
> hp_init_bios_package_attribute() validates obj->package.count and then
> calls one of the five hp_populate_*_package_data() wrappers (string,
> integer, enumeration, ordered list, password). Each wrapper forwards a
> count to its hp_populate_*_elements_from_package() parser, but instead
> of forwarding the validated obj->package.count it derives the count
> from elements[0]. elements[0] is the NAME field and is always an
> ACPI_TYPE_STRING, so reading ->package.count from it in fact reads
> ->string.length through the union acpi_object. The parsers thus bound
> themselves against the length of the name string rather than against
> the real number of elements in the package.
>
> This is safe today because hp_init_bios_package_attribute() refuses any
> package that has fewer than the type's element count, so a parser only
> ever runs on a full package and never reads past it regardless of the
> bogus bound.
>
> An upcoming change relaxes that check to accept shorter packages. Once
> a parser can receive fewer elements than its per-type count, a bound
> taken from the name length no longer reflects the array size, and the
> "elem < count" loop conditions and "elem + n >= count" sub-loop guards
> read past the end of elements[] - an out-of-bounds heap read.
>
> Forward the validated obj->package.count to every *_package_data()
> wrapper so the parsers bound themselves against the real package size.
> This does not change behaviour for the packages that enumerate
> correctly today and is a prerequisite for accepting shorter packages
> safely.
>
> 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 | 5 +++--
> drivers/platform/x86/hp/hp-bioscfg/passwdobj-attributes.c | 5 +++--
> drivers/platform/x86/hp/hp-bioscfg/string-attributes.c | 3 ++-
> 7 files changed, 22 insertions(+), 7 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..83ddf99f93954 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/order-list-attributes.c
> @@ -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,
These new arguments trigger warnings for missing kerneldoc each.
--
i.
> 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.
next prev parent reply other threads:[~2026-07-09 13:27 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-08 15:48 [PATCH v4 0/4] platform/x86: hp-bioscfg: fix ACPI package handling on HP EliteBook 840 G2 Muhammad Bilal
2026-07-08 15:48 ` [PATCH v4 1/4] platform/x86: hp-bioscfg: pass validated element count to package parsers Muhammad Bilal
2026-07-09 13:26 ` Ilpo Järvinen [this message]
2026-07-08 15:48 ` [PATCH v4 2/4] platform/x86: hp-bioscfg: bound ordered-list parsing by the package count Muhammad Bilal
2026-07-08 15:48 ` [PATCH v4 3/4] platform/x86: hp-bioscfg: accept reduced ACPI packages from older HP BIOS Muhammad Bilal
2026-07-08 15:48 ` [PATCH v4 4/4] platform/x86: hp-bioscfg: warn on element type mismatch instead of failing Muhammad Bilal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86309036-a9b3-fa41-0abd-f4eaad6b53fa@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=W_Armin@gmx.de \
--cc=hansg@kernel.org \
--cc=jorge.lopez2@hp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@weissschuh.net \
--cc=meatuni001@gmail.com \
--cc=platform-driver-x86@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=superm1@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox