[v1] ACPICA: Fix memory leak at acpi_ds_create_field()

[PATCH] ACPICA: Fix memory leak at acpi_ds_create_field()

Posted by Breno Leitao 1 month, 2 weeks ago

acpi_ds_create_field() allocates internal_pcc_buffer for operation
regions in the PCC address space.

The function runs once per field, so a second field on the
same region overwrites the pointer and orphans the previous buffer.

The matching ACPI_FREE() in acpi_ut_delete_internal_obj() only ever sees
the last one.

An arm64 platform whose SSDT declares regions with multiple fields each
leaks two buffers at boot, as detected by kmemleak:

  unreferenced object 0xffff000051e65400 (size 192):
    comm "swapper/0", pid 1, jiffies 4294668436
    backtrace (crc 0):
      __kmalloc_noprof
      acpi_ds_create_field
      acpi_ds_load2_end_op
      acpi_ds_exec_end_op
      acpi_ps_parse_loop
      acpi_ps_parse_aml
      acpi_ns_load_table
      acpi_load_tables
      acpi_init

Do not allocate a region if it was previously allocated before,
preserving the current design, but, not leaking memory.

Fixes: 0acf24ad7e10 ("ACPICA: Add support for PCC Opregion special context data")
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/acpi/acpica/dsfield.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpica/dsfield.c b/drivers/acpi/acpica/dsfield.c
index d7d56cc600d31..f7e503bf57c87 100644
--- a/drivers/acpi/acpica/dsfield.c
+++ b/drivers/acpi/acpica/dsfield.c
@@ -522,7 +522,8 @@ acpi_ds_create_field(union acpi_parse_object *op,
 	}
 
 	if (info.region_node->object->region.space_id ==
-	    ACPI_ADR_SPACE_PLATFORM_COMM) {
+	    ACPI_ADR_SPACE_PLATFORM_COMM &&
+	    !region_node->object->field.internal_pcc_buffer) {
 		region_node->object->field.internal_pcc_buffer =
 		    ACPI_ALLOCATE_ZEROED(info.region_node->object->region.
 					 length);

---
base-commit: 28d012efb4327f9c75d5e042a7c91e9a542efa98
change-id: 20260813-acpica-pcc-field-leak-8f676f55179b

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH] ACPICA: Fix memory leak at acpi_ds_create_field()

Posted by Rafael J. Wysocki (Intel) 1 month, 2 weeks ago

On Thu, Aug 13, 2026 at 3:35 PM Breno Leitao <leitao@debian.org> wrote:
>
> acpi_ds_create_field() allocates internal_pcc_buffer for operation
> regions in the PCC address space.
>
> The function runs once per field, so a second field on the
> same region overwrites the pointer and orphans the previous buffer.
>
> The matching ACPI_FREE() in acpi_ut_delete_internal_obj() only ever sees
> the last one.
>
> An arm64 platform whose SSDT declares regions with multiple fields each
> leaks two buffers at boot, as detected by kmemleak:
>
>   unreferenced object 0xffff000051e65400 (size 192):
>     comm "swapper/0", pid 1, jiffies 4294668436
>     backtrace (crc 0):
>       __kmalloc_noprof
>       acpi_ds_create_field
>       acpi_ds_load2_end_op
>       acpi_ds_exec_end_op
>       acpi_ps_parse_loop
>       acpi_ps_parse_aml
>       acpi_ns_load_table
>       acpi_load_tables
>       acpi_init
>
> Do not allocate a region if it was previously allocated before,
> preserving the current design, but, not leaking memory.
>
> Fixes: 0acf24ad7e10 ("ACPICA: Add support for PCC Opregion special context data")
> Signed-off-by: Breno Leitao <leitao@debian.org>

As per Documentation/driver-api/acpi/linuxized-acpica.rst, please
submit a pull request with this change to the upstream ACPICA project
on GitHub.  The Linux patch will become applicable when there is a
corresponding commit upstream.

> ---
>  drivers/acpi/acpica/dsfield.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/acpica/dsfield.c b/drivers/acpi/acpica/dsfield.c
> index d7d56cc600d31..f7e503bf57c87 100644
> --- a/drivers/acpi/acpica/dsfield.c
> +++ b/drivers/acpi/acpica/dsfield.c
> @@ -522,7 +522,8 @@ acpi_ds_create_field(union acpi_parse_object *op,
>         }
>
>         if (info.region_node->object->region.space_id ==
> -           ACPI_ADR_SPACE_PLATFORM_COMM) {
> +           ACPI_ADR_SPACE_PLATFORM_COMM &&
> +           !region_node->object->field.internal_pcc_buffer) {
>                 region_node->object->field.internal_pcc_buffer =
>                     ACPI_ALLOCATE_ZEROED(info.region_node->object->region.
>                                          length);
>
> ---
> base-commit: 28d012efb4327f9c75d5e042a7c91e9a542efa98
> change-id: 20260813-acpica-pcc-field-leak-8f676f55179b
>
> Best regards,
> --
> Breno Leitao <leitao@debian.org>
>

Re: [PATCH] ACPICA: Fix memory leak at acpi_ds_create_field()

Posted by Matthew Wood 4 days, 8 hours ago

On Thu, Aug 13, 2026 at 03:39:37PM +0200, Rafael J. Wysocki (Intel) wrote:
> On Thu, Aug 13, 2026 at 3:35 PM Breno Leitao <leitao@debian.org> wrote:
> >
> > acpi_ds_create_field() allocates internal_pcc_buffer for operation
> > regions in the PCC address space.
> >
> > The function runs once per field, so a second field on the
> > same region overwrites the pointer and orphans the previous buffer.
> >
> > The matching ACPI_FREE() in acpi_ut_delete_internal_obj() only ever sees
> > the last one.
> >
> > An arm64 platform whose SSDT declares regions with multiple fields each
> > leaks two buffers at boot, as detected by kmemleak:
> >
> >   unreferenced object 0xffff000051e65400 (size 192):
> >     comm "swapper/0", pid 1, jiffies 4294668436
> >     backtrace (crc 0):
> >       __kmalloc_noprof
> >       acpi_ds_create_field
> >       acpi_ds_load2_end_op
> >       acpi_ds_exec_end_op
> >       acpi_ps_parse_loop
> >       acpi_ps_parse_aml
> >       acpi_ns_load_table
> >       acpi_load_tables
> >       acpi_init
> >
> > Do not allocate a region if it was previously allocated before,
> > preserving the current design, but, not leaking memory.
> >
> > Fixes: 0acf24ad7e10 ("ACPICA: Add support for PCC Opregion special context data")
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> 
> As per Documentation/driver-api/acpi/linuxized-acpica.rst, please
> submit a pull request with this change to the upstream ACPICA project
> on GitHub.  The Linux patch will become applicable when there is a
> corresponding commit upstream.
> 

Hi Rafael,

This change has been submitted, approved, and merged in GitHub:
https://github.com/open-acpica/acpica/commit/c71849d476bb5f97ca1867849e893368e12a69e6

Can you please review here as well? Thank you.

Regards,
Matthew

> > ---
> >  drivers/acpi/acpica/dsfield.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/acpica/dsfield.c b/drivers/acpi/acpica/dsfield.c
> > index d7d56cc600d31..f7e503bf57c87 100644
> > --- a/drivers/acpi/acpica/dsfield.c
> > +++ b/drivers/acpi/acpica/dsfield.c
> > @@ -522,7 +522,8 @@ acpi_ds_create_field(union acpi_parse_object *op,
> >         }
> >
> >         if (info.region_node->object->region.space_id ==
> > -           ACPI_ADR_SPACE_PLATFORM_COMM) {
> > +           ACPI_ADR_SPACE_PLATFORM_COMM &&
> > +           !region_node->object->field.internal_pcc_buffer) {
> >                 region_node->object->field.internal_pcc_buffer =
> >                     ACPI_ALLOCATE_ZEROED(info.region_node->object->region.
> >                                          length);
> >
> > ---
> > base-commit: 28d012efb4327f9c75d5e042a7c91e9a542efa98
> > change-id: 20260813-acpica-pcc-field-leak-8f676f55179b
> >
> > Best regards,
> > --
> > Breno Leitao <leitao@debian.org>
> >

Re: [PATCH] ACPICA: Fix memory leak at acpi_ds_create_field()

Posted by Rafael J. Wysocki (Intel) 4 days, 8 hours ago

On Thu, Sep 24, 2026 at 6:30 PM Matthew Wood <thepacketgeek@gmail.com> wrote:
>
> On Thu, Aug 13, 2026 at 03:39:37PM +0200, Rafael J. Wysocki (Intel) wrote:
> > On Thu, Aug 13, 2026 at 3:35 PM Breno Leitao <leitao@debian.org> wrote:
> > >
> > > acpi_ds_create_field() allocates internal_pcc_buffer for operation
> > > regions in the PCC address space.
> > >
> > > The function runs once per field, so a second field on the
> > > same region overwrites the pointer and orphans the previous buffer.
> > >
> > > The matching ACPI_FREE() in acpi_ut_delete_internal_obj() only ever sees
> > > the last one.
> > >
> > > An arm64 platform whose SSDT declares regions with multiple fields each
> > > leaks two buffers at boot, as detected by kmemleak:
> > >
> > >   unreferenced object 0xffff000051e65400 (size 192):
> > >     comm "swapper/0", pid 1, jiffies 4294668436
> > >     backtrace (crc 0):
> > >       __kmalloc_noprof
> > >       acpi_ds_create_field
> > >       acpi_ds_load2_end_op
> > >       acpi_ds_exec_end_op
> > >       acpi_ps_parse_loop
> > >       acpi_ps_parse_aml
> > >       acpi_ns_load_table
> > >       acpi_load_tables
> > >       acpi_init
> > >
> > > Do not allocate a region if it was previously allocated before,
> > > preserving the current design, but, not leaking memory.
> > >
> > > Fixes: 0acf24ad7e10 ("ACPICA: Add support for PCC Opregion special context data")
> > > Signed-off-by: Breno Leitao <leitao@debian.org>
> >
> > As per Documentation/driver-api/acpi/linuxized-acpica.rst, please
> > submit a pull request with this change to the upstream ACPICA project
> > on GitHub.  The Linux patch will become applicable when there is a
> > corresponding commit upstream.
> >
>
> Hi Rafael,
>
> This change has been submitted, approved, and merged in GitHub:
> https://github.com/open-acpica/acpica/commit/c71849d476bb5f97ca1867849e893368e12a69e6
>
> Can you please review here as well? Thank you.

So please resend it with a Link: tag pointing to the upstream ACPICA commit.

Re: [PATCH] ACPICA: Fix memory leak at acpi_ds_create_field()

Posted by Breno Leitao 1 month, 2 weeks ago

On Thu, Aug 13, 2026 at 03:39:37PM +0200, Rafael J. Wysocki (Intel) wrote:
> > Do not allocate a region if it was previously allocated before,
> > preserving the current design, but, not leaking memory.
> >
> > Fixes: 0acf24ad7e10 ("ACPICA: Add support for PCC Opregion special context data")
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> 
> As per Documentation/driver-api/acpi/linuxized-acpica.rst, please
> submit a pull request with this change to the upstream ACPICA project
> on GitHub.  The Linux patch will become applicable when there is a
> corresponding commit upstream.

Ack. I will do it. sorry for the noise.

--breno