Re: [PATCH RFC v4 04/12] clk: zte: Add Clock registration infrastructure.

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Use-After-Free Vulnerability due to tying `struct auxiliary_device` memory lifetime to devres.
- [High] Invalid pointer dereference due to incorrect error checking of `device_node_to_regmap()`.
--

commit d5b040629c9146bc0aaf0162d247a5520ba7ab60
Author: Stefan Dösinger <stefandoesinger@gmail.com>

clk: zte: Add Clock registration infrastructure.

The next patches will implement the regmap clocks and PLL driver. The
actual hardware specific clock listing will live in a separate module.

> diff --git a/drivers/clk/zte/clk-zx.c b/drivers/clk/zte/clk-zx.c
> new file mode 100644
> index 0000000000000..6e21c4a82a46c
> --- /dev/null
> +++ b/drivers/clk/zte/clk-zx.c
> @@ -0,0 +1,157 @@
[ ... ]
> +static void zx_adev_release(struct device *dev)
> +{
> +	dev_info(dev, "Aux device released.\n");
> +}

[Severity: High]
Does this release callback need to free the memory for the auxiliary device?

If struct auxiliary_device is allocated with devres, its memory is freed
when devres triggers during unbind. This might happen before this callback
is invoked if other subsystems hold a reference to the device, potentially
causing a use-after-free.

> +
> +static void zx_adev_unregister(void *data)
> +{
> +	struct auxiliary_device *adev = data;
> +
> +	auxiliary_device_delete(adev);
> +	auxiliary_device_uninit(adev);
> +}

[Severity: High]
While this devres action decreases the device reference count, does devres
unconditionally free the adev memory right after this action returns? This
could lead to a use-after-free if lingering references are held.

> +
> +int zx_clk_probe(struct platform_device *pdev)
> +{
> +	unsigned int public_clk_count = 1, highest_id = 0;
> +	struct clk_hw_onecell_data *clocks;
> +	struct device *dev = &pdev->dev;
> +	const struct zx_clk_data *data;
> +	struct auxiliary_device *adev;
> +	struct regmap *map;
> +	struct clk *clk;
> +	unsigned int i;
> +	int res;
> +
> +	data = device_get_match_data(dev);
> +	if (!data)
> +		return -EINVAL;
> +
> +	map = device_node_to_regmap(dev->of_node);
> +	if (!map)

[Severity: High]
Since device_node_to_regmap() returns an ERR_PTR on failure rather than
NULL, should this check use IS_ERR(map)? Treating an error pointer as a
valid pointer could lead to a kernel panic when it is later dereferenced by
the clock framework.

> +		return -EINVAL;
> +
[ ... ]
> +	res = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clocks);
> +	if (res)
> +		return res;
> +
> +	adev = devm_kzalloc(dev, sizeof(*adev), GFP_KERNEL);

[Severity: High]
Does allocating adev with devm_kzalloc() improperly tie the lifetime of
the embedded struct device to devres?

Since struct device is reference counted, could an external reference
outlive the devres unbind phase, resulting in a use-after-free when
zx_adev_release() is finally called on the freed memory?

> +	if (!adev)
> +		return -ENOMEM;
> +
> +	adev->name = data->reset_auxdev_name;
> +	adev->dev.parent = dev;
> +	adev->dev.release = zx_adev_release;
> +	adev->dev.of_node = dev->of_node;
> +
> +	res = auxiliary_device_init(adev);
> +	if (res)
> +		return dev_err_probe(dev, res, "Failed to init aux dev %s\n", adev->name);
> +
> +	res = auxiliary_device_add(adev);
> +	if (res) {
> +		auxiliary_device_uninit(adev);
> +		return dev_err_probe(dev, res, "Failed to add aux dev %s\n", adev->name);
> +	}
> +
> +	return devm_add_action_or_reset(dev, zx_adev_unregister, adev);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-zx29clk-v4-0-ca994bd22e9d@gmail.com?part=4