Re: [PATCH] i2c: bcm2835: Make sure clk_init_data is fully initialized

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but a `clk_lookup` structure is leaked and leaves a dangling pointer because `clk_hw_register_clkdev()` is used without corresponding cleanup in the driver's unbind path.
--

--- Patch [1]: [PATCH] i2c: bcm2835: Make sure clk_init_data is fully initialized ---
commit 35c539831bc7c233af46e5999f9512b0c93741f6
Author: Geert Uytterhoeven <[email protected]>

i2c: bcm2835: Make sure clk_init_data is fully initialized

This commit fully initializes the clk_init_data structure to avoid subtle
issues caused by uninitialized members. This prevents breakage when
converting drivers to different methods for specifying clock parents.

> diff --git a/drivers/i2c/busses/i2c-bcm2835.c b/drivers/i2c/busses/i2c-bcm2835.c
> index 09a077b31bfe1..0e52d76cd6baf 100644
> --- a/drivers/i2c/busses/i2c-bcm2835.c
> +++ b/drivers/i2c/busses/i2c-bcm2835.c
> @@ -164,7 +164,7 @@ static struct clk *bcm2835_i2c_register_div(struct device *dev,
>  					struct clk *mclk,
>  					struct bcm2835_i2c_dev *i2c_dev)
>  {
> -	struct clk_init_data init;
> +	struct clk_init_data init = {};

[Severity: High]
This isn't a bug introduced by this patch, but looking at the rest of
bcm2835_i2c_register_div(), it registers a clock lookup globally using
clk_hw_register_clkdev() without managed devres or manual cleanup.

If the driver is repeatedly unbound and bound, such as via sysfs,
bcm2835_i2c_remove() is called and the hardware context (priv) allocated
via devm_kzalloc() is freed. However, because the unbind path does not
call clkdev_drop(), doesn't this leave a dangling clk_hw pointer in the
global clock lookup list?

Could this cause a use-after-free if the clk framework subsequently searches
for this clock, in addition to leaking the clk_lookup structures? Would
switching to devm_clk_hw_register_clkdev() instead resolve this lifecycle
mismatch?

>  	struct clk_bcm2835_i2c *priv;
>  	char name[32];
>  	const char *mclk_name;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/9f82f37e6d6c069cd44326bcd5e5a2a8069a13a9.1787239980.git.geert+renesas@glider.be?part=1