Messages in this thread |  | | From | Ilpo Järvinen <> | | Date | Wed, 8 Jul 2026 13:58:38 +0300 (EEST) | | Subject | Re: [PATCH v3 2/4] platform/x86: hp-bioscfg: bound ordered-list parsing by the package count |
| |
On Wed, 8 Jul 2026, Muhammad Bilal wrote:
> hp_populate_ordered_list_elements_from_package() differs from the other > per-type parsers: its main loop is bounded only by the fixed per-type > count and never checks elem against the number of elements actually > present in the package, > > for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT; elem++, eloc++) > > whereas the string, integer, enumeration and password parsers bound > their main loop with "elem < count" as well. > > This is safe today because hp_init_bios_package_attribute() rejects any > package with fewer than ORD_ELEM_CNT elements before the parser runs. A > later patch relaxes that check to accept shorter packages;
An upcoming change, however, relaxes ...
> once this > loop can be handed fewer than ORD_ELEM_CNT elements it indexes > order_obj[elem] past the end of the array - an out-of-bounds heap read.
I don't think we need this part of the explanation, it will never happen as you're fixing it beforehand. :-) So please drop it as unnecessary detail.
It's pretty obvious to kernel developers anyway if there's a runaway index so there's no big need in general to tell too simple basics like that.
> Bound the loop by the validated element count as well, so it stops at > whichever comes first, the per-type count or the real package size, > > for (elem = 1, eloc = 1; eloc < ORD_ELEM_CNT && elem < order_obj_count; > elem++, eloc++) > > order_obj_count is the validated count plumbed in by the previous > patch. No functional change for packages that enumerate correctly > today. > > 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/order-list-attributes.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > 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 83ddf99f93954..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: >
-- i.
|  |