Re: [PATCH RFC v5 09/12] clk: zte: Introduce a driver for zx297520v3 matrix clocks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Double free of the `adev` structure in the error handling path of `auxiliary_device_add()`.
--

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

clk: zte: Introduce a driver for zx297520v3 matrix clocks

This commit introduces a clock driver for the zx297520v3 matrix clocks.
It registers several multiplexers, gates, and inputs by utilizing the
shared zx_clk_common_probe() initialization routine.

> diff --git a/drivers/clk/zte/clk-zx297520v3.c b/drivers/clk/zte/clk-zx297520v3.c
> index bb3d64eff9096..17e1f741b48c9 100644
> --- a/drivers/clk/zte/clk-zx297520v3.c
> +++ b/drivers/clk/zte/clk-zx297520v3.c

[ ... ]

> +static int zx297520v3_matrixclk_probe(struct platform_device *pdev)
> +{
> +	return zx_clk_common_probe(&pdev->dev, pdev->dev.parent->of_node,
> +				   &zx297520v3_matrixclk_data);
> +}

[Severity: High]
This is a pre-existing issue, but does this error path result in a double
free if auxiliary_device_add() fails during probe?

Looking at zx_clk_common_probe() in drivers/clk/zte/clk-zx.c, the error
path handles a failure from auxiliary_device_add() by jumping to
adev_uninit:

drivers/clk/zte/clk-zx.c:zx_clk_common_probe() {
    ...
    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;
}

When auxiliary_device_uninit(adev) is called, it drops the reference count,
which triggers the assigned release callback zx_adev_release(). This
callback legitimately frees the structure:

drivers/clk/zte/clk-zx.c:zx_adev_release() {
    struct auxiliary_device *adev = to_auxiliary_dev(dev);
    
    kfree(adev);
}

However, after returning from auxiliary_device_uninit(), the error path
unconditionally falls through to the adev_free label, calling kfree(adev)
a second time.

Since the structure is automatically freed in the release callback, is it safe
to fall through to kfree(adev) here, or should the manual kfree be removed?

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