1. Patchew
  2. linux
  3. View series

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

Anirudh Prasad posted 1 patch 1 month, 3 weeks ago
There is a newer version of this series
drivers/acpi/pfr_update.c | 47 ++++++++++++++++++++++-----------------
1 file changed, 26 insertions(+), 21 deletions(-)
[PATCH v3] ACPI: pfr_update: fix stack buffer overflow in query_capability()
Posted by Anirudh Prasad 1 month, 3 weeks ago 
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: stable@vger.kernel.org
Signed-off-by: Anirudh Prasad <icarus@a0rg.com>
---
 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
Re: [PATCH v3] ACPI: pfr_update: fix stack buffer overflow in query_capability()
Posted by Rafael J. Wysocki (Intel) 1 month, 2 weeks ago 
On Fri, Aug 7, 2026 at 6:01 PM Anirudh Prasad <icarus@a0rg.com> 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.
>
> 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: stable@vger.kernel.org
> Signed-off-by: Anirudh Prasad <icarus@a0rg.com>
> ---
>  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;

ret is equal to -EINVAL already at this point if I'm not mistaken, so
no need to set it again to the same value.

> +               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
>
>
Re: [PATCH v3] ACPI: pfr_update: fix stack buffer overflow in query_capability()
Posted by Anirudh Prasad 1 month, 2 weeks ago 
Hi, just gently nudging this. Thanks!


From: Anirudh Prasad <icarus@a0rg.com>
To: "linux-acpi"<linux-acpi@vger.kernel.org>
Cc: "rafaeljwysocki"<rafael.j.wysocki@intel.com>, "linux-kernel"<linux-kernel@vger.kernel.org>, "stable"<stable@vger.kernel.org>
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: stable@vger.kernel.org 
 > Signed-off-by: Anirudh Prasad <icarus@a0rg.com> 
 > --- 
 >  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 
 >  
 >

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.