Re: [PATCH v3] ACPI: pfr_update: fix stack buffer overflow in query_capability()
Anirudh Prasad <[email protected]> Thu, 13 Aug 2026 12:41:15 +0530
| Newsgroups | gmane.linux.acpi.devel,gmane.linux.kernel,gmane.linux.kernel.stable |
|---|---|
| Message-ID | <[email protected]> |
Hi, just gently nudging this. Thanks! From: Anirudh Prasad <[email protected]> To: "linux-acpi"<[email protected]> Cc: "rafaeljwysocki"<[email protected]>, "linux-kernel"<[email protected]>, "stable"<[email protected]> Date: Fri, 07 Aug 2026 21:31:12 +0530 Subject: [PATCH v3] ACPI: pfr_update: fix stack buffer overflow in query_capability() > 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. > > Introduce a helper pointer to out_obj->package.elements and use it > to validate each buffer length against its destination field size > before copying, 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 | 47 ++++++++++++++++++++++----------------- > 1 file changed, 26 insertions(+), 21 deletions(-) > > diff --git a/drivers/acpi/pfr_update.c b/drivers/acpi/pfr_update.c > index 6283105bb0e8..79cedd4cf2a2 100644 > --- a/drivers/acpi/pfr_update.c > +++ b/drivers/acpi/pfr_update.c > @@ -120,7 +120,7 @@ static int query_capability(struct pfru_update_cap_info *cap_hdr, > struct pfru_device *pfru_dev) > { > acpi_handle handle = ACPI_HANDLE(pfru_dev->parent_dev); > - union acpi_object *out_obj; > + union acpi_object *out_obj, *elem; > int ret = -EINVAL; > > out_obj = acpi_evaluate_dsm_typed(handle, &pfru_guid, > @@ -150,7 +150,9 @@ static int query_capability(struct pfru_update_cap_info *cap_hdr, > goto free_acpi_buffer; > } > > - cap_hdr->status = out_obj->package.elements[CAP_STATUS_IDX].integer.value; > + elem = out_obj->package.elements; > + > + cap_hdr->status = elem[CAP_STATUS_IDX].integer.value; > if (cap_hdr->status != DSM_SUCCEED) { > ret = -EBUSY; > dev_dbg(pfru_dev->parent_dev, "Query cap Error Status:%d\n", > @@ -158,29 +160,32 @@ static int query_capability(struct pfru_update_cap_info *cap_hdr, > goto free_acpi_buffer; > } > > - cap_hdr->update_cap = out_obj->package.elements[CAP_UPDATE_IDX].integer.value; > + if (elem[CAP_CODE_TYPE_IDX].buffer.length > sizeof(cap_hdr->code_type) || > + elem[CAP_DRV_TYPE_IDX].buffer.length > sizeof(cap_hdr->drv_type) || > + elem[CAP_PLAT_ID_IDX].buffer.length > sizeof(cap_hdr->platform_id) || > + elem[CAP_OEM_ID_IDX].buffer.length > sizeof(cap_hdr->oem_id)) { > + ret = -EINVAL; > + goto free_acpi_buffer; > + } > + > + cap_hdr->update_cap = elem[CAP_UPDATE_IDX].integer.value; > memcpy(&cap_hdr->code_type, > - out_obj->package.elements[CAP_CODE_TYPE_IDX].buffer.pointer, > - out_obj->package.elements[CAP_CODE_TYPE_IDX].buffer.length); > - cap_hdr->fw_version = > - out_obj->package.elements[CAP_FW_VER_IDX].integer.value; > - cap_hdr->code_rt_version = > - out_obj->package.elements[CAP_CODE_RT_VER_IDX].integer.value; > + elem[CAP_CODE_TYPE_IDX].buffer.pointer, > + elem[CAP_CODE_TYPE_IDX].buffer.length); > + cap_hdr->fw_version = elem[CAP_FW_VER_IDX].integer.value; > + cap_hdr->code_rt_version = elem[CAP_CODE_RT_VER_IDX].integer.value; > memcpy(&cap_hdr->drv_type, > - out_obj->package.elements[CAP_DRV_TYPE_IDX].buffer.pointer, > - out_obj->package.elements[CAP_DRV_TYPE_IDX].buffer.length); > - cap_hdr->drv_rt_version = > - out_obj->package.elements[CAP_DRV_RT_VER_IDX].integer.value; > - cap_hdr->drv_svn = > - out_obj->package.elements[CAP_DRV_SVN_IDX].integer.value; > + elem[CAP_DRV_TYPE_IDX].buffer.pointer, > + elem[CAP_DRV_TYPE_IDX].buffer.length); > + cap_hdr->drv_rt_version = elem[CAP_DRV_RT_VER_IDX].integer.value; > + cap_hdr->drv_svn = elem[CAP_DRV_SVN_IDX].integer.value; > memcpy(&cap_hdr->platform_id, > - out_obj->package.elements[CAP_PLAT_ID_IDX].buffer.pointer, > - out_obj->package.elements[CAP_PLAT_ID_IDX].buffer.length); > + elem[CAP_PLAT_ID_IDX].buffer.pointer, > + elem[CAP_PLAT_ID_IDX].buffer.length); > memcpy(&cap_hdr->oem_id, > - out_obj->package.elements[CAP_OEM_ID_IDX].buffer.pointer, > - out_obj->package.elements[CAP_OEM_ID_IDX].buffer.length); > - cap_hdr->oem_info_len = > - out_obj->package.elements[CAP_OEM_INFO_IDX].buffer.length; > + elem[CAP_OEM_ID_IDX].buffer.pointer, > + elem[CAP_OEM_ID_IDX].buffer.length); > + cap_hdr->oem_info_len = elem[CAP_OEM_INFO_IDX].buffer.length; > > ret = 0; > > -- > 2.55.0 > >