Re: [PATCH v2] ACPI: pfr_update: fix stack buffer overflow in query_capability()
"Rafael J. Wysocki (Intel)" <[email protected]> Fri, 7 Aug 2026 17:42:49 +0200
| Newsgroups | gmane.linux.acpi.devel,gmane.linux.kernel,gmane.linux.kernel.stable |
|---|---|
| Message-ID | <CAJZ5v0h-HiNNO4Cwqd_8Rs2u+05U-sM4cnektpKCWFW1NWxLLg@mail.gmail.com> |
On Fri, Aug 7, 2026 at 4:40 PM Anirudh Prasad <[email protected]> wrote: > > query_capability() copies four ACPI buffer objects returned by the > firmware _DSM into fixed-size u8[16] fields in struct > pfru_update_cap_info using memcpy with the firmware-supplied length: > > memcpy(&cap_hdr->code_type, > elements[CAP_CODE_TYPE_IDX].buffer.pointer, > elements[CAP_CODE_TYPE_IDX].buffer.length); > > The same pattern repeats for drv_type, platform_id, and oem_id. > If the firmware returns buffer.length > 16 for any of these fields, > memcpy writes past the destination array. > > struct pfru_update_cap_info is stack-allocated in pfru_ioctl(). > Confirmed with KASAN on 7.2-rc6: three stack-out-of-bounds reports > are generated when a DSM returns 64-byte buffers, with writes reaching > 44 bytes past the end of cap_hdr's [64, 156) frame window into > adjacent stack redzones. > > Fix by validating each buffer length against its destination field > size before copying, and returning -EINVAL if the firmware supplies > an oversized buffer. > > Fixes: 0db89fa243e5 ("ACPI: Introduce Platform Firmware Runtime Update device driver") > Cc: [email protected] > Signed-off-by: Anirudh Prasad <[email protected]> > --- > drivers/acpi/pfr_update.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/acpi/pfr_update.c b/drivers/acpi/pfr_update.c > index 6283105bb0e8..e14e053cea44 100644 > --- a/drivers/acpi/pfr_update.c > +++ b/drivers/acpi/pfr_update.c > @@ -158,6 +158,14 @@ static int query_capability(struct pfru_update_cap_info *cap_hdr, > goto free_acpi_buffer; > } > > + if (out_obj->package.elements[CAP_CODE_TYPE_IDX].buffer.length > sizeof(cap_hdr->code_type) || > + out_obj->package.elements[CAP_DRV_TYPE_IDX].buffer.length > sizeof(cap_hdr->drv_type) || > + out_obj->package.elements[CAP_PLAT_ID_IDX].buffer.length > sizeof(cap_hdr->platform_id) || > + out_obj->package.elements[CAP_OEM_ID_IDX].buffer.length > sizeof(cap_hdr->oem_id)) { The lines above are too long. Please introduce a helper pointer to the elements array, say "elem = out_obj->package.elements", and use if in these checks. > + ret = -EINVAL; > + goto free_acpi_buffer; > + } > + > cap_hdr->update_cap = out_obj->package.elements[CAP_UPDATE_IDX].integer.value; > memcpy(&cap_hdr->code_type, > out_obj->package.elements[CAP_CODE_TYPE_IDX].buffer.pointer, > -- > 2.55.0 > > >