Re: [PATCH] ACPI: pfr_update: fix stack buffer overflow in query_capability()

"Rafael J. Wysocki (Intel)" <[email protected]>
Newsgroups org.kernel.vger.linux-acpi,org.kernel.vger.linux-kernel,org.kernel.vger.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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.