Re: [PATCH RFC v4 06/12] clk: zte: Add regmap based clocks

[sashiko-bot@kernel.org]

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

New issues:
- [High] In `zte_clk_regmap_gate_is_enabled`, the result of `regmap_read` is ignored and the read value is not properly masked, leading to uninitialized memory use and incorrect clock state reporting.

Pre-existing issues:
- [High] Allocating `struct auxiliary_device` with `devm_kzalloc` causes a Use-After-Free when the parent device is removed while external references still exist.
--

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

clk: zte: Add regmap based clocks

This commit adds regmap based clocks for ZTE, simplifying the implementation
based on the meson clock regmap code.

> diff --git a/drivers/clk/zte/clk-regmap.c b/drivers/clk/zte/clk-regmap.c
> index 7908f1562f639..d9459417d17dd 100644
> --- a/drivers/clk/zte/clk-regmap.c
> +++ b/drivers/clk/zte/clk-regmap.c

[ ... ]

> +static int zte_clk_regmap_gate_is_enabled(struct clk_hw *hw)
> +{
> +	struct zte_clk_regmap *clk = to_zte_clk_regmap(hw);
> +	u32 val;
> +
> +	regmap_read(clk->map, clk->reg, &val);
> +	return !!val;
> +}

[Severity: High]
Does this code properly mask the specific gate bit? Since clock gates commonly
share registers, returning !!val might erroneously report this specific gate
as enabled whenever any other bit in the register is active, rather than using
BIT(clk->shift) to evaluate the correct bit.

Also, should the return value of regmap_read() be checked here? If the read
fails, val remains uninitialized, which could lead to evaluating stack
garbage.

[ ... ]

In drivers/clk/zte/clk-zx.c:

[Severity: High]
This is a pre-existing issue, but does allocating the auxiliary_device with
devm_kzalloc() in zx_clk_probe() create a use-after-free risk?

If the parent device is unbound, devres will automatically free the memory
for the auxiliary_device. However, if external references to the embedded
device still exist, the memory will be freed while it is still in use.

When the final reference is eventually dropped, the kernel will attempt to
call the release callback on the freed memory:

zx_adev_release() {
        dev_info(dev, "Aux device released.\n");
}

Should the structure be allocated with kzalloc() instead, and explicitly
freed inside the release callback?

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