[PATCH v1 05/26] ACPICA: Fix NULL pointer dereference in acpi_ev_address_space_dispatch()

Posted by Rafael J. Wysocki 3 months ago

From: Alexey Simakov <bigalex934@gmail.com>

Cover a missed execution path with a new check.

Fixes: 0acf24ad7e10 ("ACPICA: Add support for PCC Opregion special context data")
Link: https://github.com/acpica/acpica/commit/f421dd9dd897
Signed-off-by: Alexey Simakov <bigalex934@gmail.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpica/evregion.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpica/evregion.c b/drivers/acpi/acpica/evregion.c
index fa3475da7ea9..b6198f73c81d 100644
--- a/drivers/acpi/acpica/evregion.c
+++ b/drivers/acpi/acpica/evregion.c
@@ -163,7 +163,9 @@ acpi_ev_address_space_dispatch(union acpi_operand_object *region_obj,
 			return_ACPI_STATUS(AE_NOT_EXIST);
 		}
 
-		if (region_obj->region.space_id == ACPI_ADR_SPACE_PLATFORM_COMM) {
+		if (field_obj
+		    && region_obj->region.space_id ==
+		    ACPI_ADR_SPACE_PLATFORM_COMM) {
 			struct acpi_pcc_info *ctx =
 			    handler_desc->address_space.context;
 
-- 
2.51.0

Re: [PATCH v1 05/26] ACPICA: Fix NULL pointer dereference in acpi_ev_address_space_dispatch()

Posted by Guenter Roeck 1 month ago

Hi,

On Wed, Jan 14, 2026 at 01:20:17PM +0100, Rafael J. Wysocki wrote:
> From: Alexey Simakov <bigalex934@gmail.com>
> 
> Cover a missed execution path with a new check.
> 
> Fixes: 0acf24ad7e10 ("ACPICA: Add support for PCC Opregion special context data")
> Link: https://github.com/acpica/acpica/commit/f421dd9dd897
> Signed-off-by: Alexey Simakov <bigalex934@gmail.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/acpi/acpica/evregion.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/acpica/evregion.c b/drivers/acpi/acpica/evregion.c
> index fa3475da7ea9..b6198f73c81d 100644
> --- a/drivers/acpi/acpica/evregion.c
> +++ b/drivers/acpi/acpica/evregion.c
> @@ -163,7 +163,9 @@ acpi_ev_address_space_dispatch(union acpi_operand_object *region_obj,
>  			return_ACPI_STATUS(AE_NOT_EXIST);
>  		}
>  
> -		if (region_obj->region.space_id == ACPI_ADR_SPACE_PLATFORM_COMM) {
> +		if (field_obj
> +		    && region_obj->region.space_id ==
> +		    ACPI_ADR_SPACE_PLATFORM_COMM) {
>  			struct acpi_pcc_info *ctx =
>  			    handler_desc->address_space.context;
>  
Google's experimental AI review agent provided the following feedback:

 If this setup block is executed with a NULL `field_obj`, it will skip
 initializing `ctx->length` and `ctx->subspace_id` even though they do not
 depend on `field_obj`.

 Additionally, because this initialization is part of the
 `!(region_obj->region.flags & AOPOBJ_SETUP_COMPLETE)` block, the setup flag
 will be set shortly after this. Does this mean that if the first call has a
 NULL `field_obj`, the region will be marked as setup complete, and a
 subsequent call with a valid `field_obj` will never initialize
 `ctx->internal_buffer`? Should the `field_obj` check only guard the
 assignment of `ctx->internal_buffer`?

Please let me know if this is a real concern or not to help improve the agent.

Thanks,
Guenter

Re: [PATCH v1 05/26] ACPICA: Fix NULL pointer dereference in acpi_ev_address_space_dispatch()

Posted by Rafael J. Wysocki 4 weeks, 1 day ago

On Mon, Mar 16, 2026 at 6:46 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> Hi,
>
> On Wed, Jan 14, 2026 at 01:20:17PM +0100, Rafael J. Wysocki wrote:
> > From: Alexey Simakov <bigalex934@gmail.com>
> >
> > Cover a missed execution path with a new check.
> >
> > Fixes: 0acf24ad7e10 ("ACPICA: Add support for PCC Opregion special context data")
> > Link: https://github.com/acpica/acpica/commit/f421dd9dd897
> > Signed-off-by: Alexey Simakov <bigalex934@gmail.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/acpi/acpica/evregion.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/acpica/evregion.c b/drivers/acpi/acpica/evregion.c
> > index fa3475da7ea9..b6198f73c81d 100644
> > --- a/drivers/acpi/acpica/evregion.c
> > +++ b/drivers/acpi/acpica/evregion.c
> > @@ -163,7 +163,9 @@ acpi_ev_address_space_dispatch(union acpi_operand_object *region_obj,
> >                       return_ACPI_STATUS(AE_NOT_EXIST);
> >               }
> >
> > -             if (region_obj->region.space_id == ACPI_ADR_SPACE_PLATFORM_COMM) {
> > +             if (field_obj
> > +                 && region_obj->region.space_id ==
> > +                 ACPI_ADR_SPACE_PLATFORM_COMM) {
> >                       struct acpi_pcc_info *ctx =
> >                           handler_desc->address_space.context;
> >
> Google's experimental AI review agent provided the following feedback:
>
>  If this setup block is executed with a NULL `field_obj`, it will skip
>  initializing `ctx->length` and `ctx->subspace_id` even though they do not
>  depend on `field_obj`.
>
>  Additionally, because this initialization is part of the
>  `!(region_obj->region.flags & AOPOBJ_SETUP_COMPLETE)` block, the setup flag
>  will be set shortly after this. Does this mean that if the first call has a
>  NULL `field_obj`, the region will be marked as setup complete, and a
>  subsequent call with a valid `field_obj` will never initialize
>  `ctx->internal_buffer`? Should the `field_obj` check only guard the
>  assignment of `ctx->internal_buffer`?

Thanks for the report!

> Please let me know if this is a real concern or not to help improve the agent.

Saket, Pawel, can you please have a look at this?