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

[Rafael J. Wysocki (Intel)]

On Thu, Aug 6, 2026 at 3:41 PM Anirudh Prasad <icarus@xxxxxxxx> 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: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Anirudh Prasad <icarus@xxxxxxxx>

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
>