Re: [PATCH] ACPI: pfr_update: fix stack buffer overflow in query_capability()
"Rafael J. Wysocki (Intel)" <[email protected]> Thu, 6 Aug 2026 16:05:12 +0200
| Newsgroups | gmane.linux.acpi.devel,gmane.linux.kernel,gmane.linux.kernel.stable |
|---|---|
| Message-ID | <CAJZ5v0juEQMUPmGyo4R6LGK01fTR4DHz3YrSH+7YFFFogiudhQ@mail.gmail.com> |
On Thu, Aug 6, 2026 at 3:41 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 clamping each memcpy length to the size of its destination > field with min_t(u32, buffer.length, sizeof(cap_hdr->field)). > > Fixes: 0db89fa243e5 ("ACPI: Introduce Platform Firmware Runtime Update device driver") > Cc: [email protected] > Signed-off-by: Anirudh Prasad <[email protected]> If the firmware wants the kernel to copy more data than the latter has room for, the operation should fail instead of pretending to succeed. > --- > drivers/acpi/pfr_update.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/acpi/pfr_update.c b/drivers/acpi/pfr_update.c > index 6283105bb0e8..0219347cd78f 100644 > --- a/drivers/acpi/pfr_update.c > +++ b/drivers/acpi/pfr_update.c > @@ -161,24 +161,28 @@ static int query_capability(struct pfru_update_cap_info *cap_hdr, > 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, > - out_obj->package.elements[CAP_CODE_TYPE_IDX].buffer.length); > + min_t(u32, out_obj->package.elements[CAP_CODE_TYPE_IDX].buffer.length, > + sizeof(cap_hdr->code_type))); > 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; > 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); > + min_t(u32, out_obj->package.elements[CAP_DRV_TYPE_IDX].buffer.length, > + sizeof(cap_hdr->drv_type))); > 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; > 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); > + min_t(u32, out_obj->package.elements[CAP_PLAT_ID_IDX].buffer.length, > + sizeof(cap_hdr->platform_id))); > 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); > + min_t(u32, out_obj->package.elements[CAP_OEM_ID_IDX].buffer.length, > + sizeof(cap_hdr->oem_id))); > cap_hdr->oem_info_len = > out_obj->package.elements[CAP_OEM_INFO_IDX].buffer.length; > > -- > 2.55.0 >