Re: [RFC PATCH v1 3/8] misc/arm-cla: Probe firmware-described devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Improper handling of `krealloc_array()` failure in `cla_dev_domain_get()` overwrites the array pointer with NULL, causing a memory leak and subsequent NULL pointer dereference.
- [High] Missing `platform_driver_unregister()` on the `cla_domains_finalise()` error path leaves the driver registered, leading to a Use-After-Free.
- [Medium] Integer overflow when calculating `cla_nr_domains` causes lookup tables to remain unpopulated, silently breaking device initialization.
--

--- Patch [3]: [RFC PATCH v1 3/8] misc/arm-cla: Probe firmware-described devices ---
commit f4618493e3d162e6d466cff0e6d759171e5655f5
Author: Jean-Philippe Brucker <jpb@kernel.org>

misc/arm-cla: Probe firmware-described devices

Discover CLA devices from firmware and instantiate the driver with
platform_driver_probe(). Map the MMIO frame, select the kernel-accessible
privilege-level frame and associate each CLA with the CPU named by firmware.

> diff --git a/drivers/misc/arm-cla/cla-init.c b/drivers/misc/arm-cla/cla-init.c
> --- a/drivers/misc/arm-cla/cla-init.c
> +++ b/drivers/misc/arm-cla/cla-init.c

[ ... ]

>  	ret = platform_driver_probe(&cla_driver, cla_probe);
>  	if (ret) {
>  		if (ret != -ENODEV)
>  			pr_err("arm-cla: probe failed with %d\n", ret);
>  		/* Some domains may have been created during probe */
>  		goto err_domains_free;
>  	}
>  
>  	ret = cla_domains_finalise();
>  	if (ret) {
>  		pr_err("arm-cla: failed to finalise domains: %d", ret);
>  		goto err_domains_free;

[Severity: High]
If cla_domains_finalise() fails, this jumps to err_domains_free, which skips
platform_driver_unregister(). Will this leave dangling pointers to the freed
cla_driver struct on the platform bus?

>  	}
>  
>  	/*
>  	 * Each CPU initializes their own CLA. CPUHP uses a pair of smp_mb()
>  	 * when calling the startup callback, ensuring that cla_dev_setup()
>  	 * reads fully initialized cla_lut_cpu and cla_dev structures.
>  	 */
>  	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "arm-cla",
>  				cla_dev_setup, cla_dev_teardown);
>  	if (ret < 0) {
>  		pr_err("arm-cla: failed to setup cpuhp: %d", ret);
>  		goto err_driver_unregister;
>  	}
>  	cla_cpuhp_state = ret;
>  
>  	return 0;
>  
>  err_driver_unregister:
>  	platform_driver_unregister(&cla_driver);
>  err_domains_free:
>  	cla_domains_free();
>  	return ret;
>  }

[ ... ]

> diff --git a/drivers/misc/arm-cla/cla-topology.c b/drivers/misc/arm-cla/cla-topology.c
> --- /dev/null
> +++ b/drivers/misc/arm-cla/cla-topology.c

[ ... ]

> +struct cla_domain *cla_dev_domain_get(struct cla_dev *dev)
> +{
> +	int ret;
> +	unsigned int domain_id;
> +	struct cla_domain *domain;
> +
> +	/* Domain ID is provided by firmware */
> +	ret = of_property_read_u32(dev->dev->of_node, "domain", &domain_id);
> +	if (WARN_ON(ret))
> +		return ERR_PTR(-EINVAL);
> +
> +	domain = xa_load(&cla_domains, domain_id);
> +	if (domain) {
> +		domain->nr_devs++;
> +		domain->devs = krealloc_array(domain->devs, domain->nr_devs,
> +					      sizeof(*domain->devs), GFP_KERNEL);

[Severity: High]
Does this code leak the original domain->devs array if krealloc_array() fails
and returns NULL? Also, since domain->nr_devs was unconditionally incremented,
will a subsequent call to cla_domains_finalise() dereference the NULL
pointer at domain->devs[j]?

> +		if (!domain->devs)
> +			return ERR_PTR(-ENOMEM);
> +		domain->devs[domain->nr_devs - 1] = dev;
> +		return domain;
> +	}
> +
> +	domain = cla_domain_alloc(dev, domain_id);
> +	if (IS_ERR(domain))
> +		return domain;
> +
> +	cla_nr_domains = max(domain_id + 1, cla_nr_domains);

[Severity: Medium]
Can this calculation overflow to 0 if firmware provides a domain_id of
UINT_MAX? If it overflows, will cla_domains_finalise() skip its loop and
leave the device uninitialized in the lookup tables?

> +
> +	return domain;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717104759.123203-1-ryan.roberts@arm.com?part=3