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

[Jean-Philippe Brucker <jpb@kernel.org>]

On Fri, Jul 17, 2026 at 10:57:08AM +0000, sashiko-bot@kernel.org wrote:
> > 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?

Yes, needs fix

> Also, since domain->nr_devs was unconditionally incremented,
> will a subsequent call to cla_domains_finalise() dereference the NULL
> pointer at domain->devs[j]?

No subsequent call

> 
> > +		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?

FW does not provide UINT_MAX as domain ID