Thank you for your contribution! Sashiko AI review found 2 potential issue(s) 
to consider:
- [High] acpi: cppc: dangling per-CPU pointer after exit leads to UAF
- [Medium] acpi/cppc: missing kobject_del() causes sysfs -EEXIST on driver 
re-bind
--

commit 1405c81459a62da53ac116a49a65cbdc3bcdcd68
Author: Ian Rogers <[email protected]>

ACPI: CPPC: Free cpc_desc in kobject release callback

cpc_desc_ptr embeds a struct kobject that is registered in sysfs via
kobject_init_and_add(), but cppc_ktype lacked a .release callback and
acpi_cppc_processor_exit() freed cpc_ptr immediately after kobject_put().
This patch moves the sys_mem_vaddr unmapping and kfree(cpc_ptr) into a new
cppc_release() callback.

> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index f370be8715ae2..0fefe1cdb60ce 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> [ ... ]
> @@ -961,15 +966,7 @@ void acpi_cppc_processor_exit(struct acpi_processor *pr)
>       if (!cpc_ptr)
>               return;
>  
> -     /* Free all the mapped sys mem areas for this CPU */
> -     for (i = 2; i < cpc_ptr->num_entries; i++) {
> -             addr = cpc_ptr->cpc_regs[i-2].sys_mem_vaddr;
> -             if (addr)
> -                     iounmap(addr);
> -     }
> -
>       kobject_put(&cpc_ptr->kobj);

[Severity: High]
Does this leave a dangling pointer in the per-CPU variable?

When acpi_cppc_processor_exit() drops the kobject refcount, the underlying
memory is freed. However, it seems per_cpu(cpc_desc_ptr, pr->id) is never
set to NULL.

Could other CPPC functions that rely on this pointer being NULL if
uninitialized, such as cppc_get_perf_caps():

        struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
        // ...
        if (!cpc_desc) {
                pr_debug("No CPC descriptor for CPU:%d\n", cpunum);
                return -ENODEV;
        }

successfully pass the check on the dangling pointer and trigger a
use-after-free upon driver unbind?

[Severity: Medium]
Could omitting kobject_del() here cause a sysfs -EEXIST error when the
driver is re-bound?

The teardown path drops the reference but appears to omit explicitly
unregistering the sysfs directory. If the kobject's refcount does not
immediately reach 0 (for example, if userspace has a CPPC sysfs file open,
or if CONFIG_DEBUG_KOBJECT_RELEASE=y defers the release), the sysfs
directory is not synchronously removed.

Should kobject_del() be called before kobject_put() to ensure a subsequent
kobject_add() in the probe path does not collide with a lingering sysfs
directory?

> -     kfree(cpc_ptr);
>  }
>  EXPORT_SYMBOL_GPL(acpi_cppc_processor_exit);

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=5

Reply via email to