Re: [PATCH RFC v5 05/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] Double free of the auxiliary device structure on the `auxiliary_device_add()` error path.
- [Medium] Passing a valid pointer to `dev_err_probe` instead of the actual error code on `clk_prepare_enable()` failure.
--

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

clk: zte: Add Clock registration infrastructure.

This patch adds common clock registration infrastructure for ZTE SoCs,
implementing the registration and probe routines for various clock types
and preparing for the regmap clocks and PLL drivers.

> diff --git a/drivers/clk/zte/clk-zx.c b/drivers/clk/zte/clk-zx.c
> --- /dev/null
> +++ b/drivers/clk/zte/clk-zx.c
[ ... ]
> +int zx_clk_common_probe(struct device *dev, struct device_node *of_node,
> +			const struct zx_clk_data *data)
> +{
[ ... ]
> +		res = clk_prepare_enable(clk);
> +		if (res) {
> +			clk_put(clk);
> +			return dev_err_probe(dev, PTR_ERR(clk), "Input clk %s enable failure\n",
> +					     data->inputs_enable[i]);
> +		}

[Severity: Medium]
Should this pass the actual error code res to dev_err_probe instead of
PTR_ERR(clk)?

Since clk is a valid pointer here (having successfully returned from
of_clk_get_by_name earlier), PTR_ERR(clk) will return a casted kernel address
rather than the actual error code.

[ ... ]
> +	res = auxiliary_device_add(adev);
> +	if (res) {
> +		dev_err_probe(dev, res, "Failed to add aux dev %s\n", adev->name);
> +		goto adev_uninit;
> +	}
> +
> +	return devm_add_action_or_reset(dev, zx_adev_unregister, adev);
> +
> +adev_uninit:
> +	auxiliary_device_uninit(adev);
> +adev_free:
> +	kfree(adev);
> +	return res;
> +}

[Severity: High]
Will this result in a double free if auxiliary_device_add fails?

When jumping to adev_uninit, auxiliary_device_uninit drops the reference
count to 0, which triggers the zx_adev_release callback:

static void zx_adev_release(struct device *dev)
{
	struct auxiliary_device *adev = to_auxiliary_dev(dev);

	kfree(adev);
}

The code then falls through to the adev_free label and explicitly calls
kfree on the adev pointer again.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260628-zx29clk-v5-0-79ff044e4192@gmail.com?part=5