Re: [PATCH 3/3] media: samsung: exynos4-is: 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] Double free and use-after-free of device node references in OF iterator loop.
--

--- Patch [3]: [PATCH 3/3] media: samsung: exynos4-is: Make sure clk_init_data is fully initialized ---
commit f3a10ad891243d9b897aed17537077a8bb8725bc
Author: Geert Uytterhoeven <[email protected]>

    media: samsung: exynos4-is: Make sure clk_init_data is fully initialized
    
    The clk_init_data structure contains mutually-exclusive members for
    specifying clock parents. This commit ensures all members are fully
    initialized to zero, preventing subtle bugs with uninitialized data.

> diff --git a/drivers/media/platform/samsung/exynos4-is/media-dev.c b/drivers/media/platform/samsung/exynos4-is/media-dev.c
> index 2e08c4435859e..638babf528221 100644
> --- a/drivers/media/platform/samsung/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/samsung/exynos4-is/media-dev.c
> @@ -1313,7 +1313,7 @@ static int fimc_md_register_clk_provider(struct fimc_md *fmd)
>  
>  	for (i = 0; i < FIMC_MAX_CAMCLKS; i++) {
>  		struct cam_clk *camclk = &cp->camclk[i];
> -		struct clk_init_data init;
> +		struct clk_init_data init = {};
>  		const char *p_name;
>  
>  		ret = of_property_read_string_index(dev->of_node,

[Severity: High]
This is a pre-existing issue, but I noticed a potential double-free of device
node references elsewhere in this file.

Looking at fimc_md_parse_port_node(), it iterates over child nodes using the
scoped iterator macro:

drivers/media/platform/samsung/exynos4-is/media-dev.c:fimc_md_parse_port_node() {
    ...
	for_each_child_of_node_scoped(port, ep) {
		ret = fimc_md_parse_one_endpoint(fmd, ep);
		if (ret < 0)
			return ret;
	}
    ...
}

This macro automatically manages the reference counting for ep, dropping it
implicitly when the loop advances or exits.

However, inside fimc_md_parse_one_endpoint(), the code explicitly calls
of_node_put(ep) on all paths:

drivers/media/platform/samsung/exynos4-is/media-dev.c:fimc_md_parse_one_endpoint() {
    ...
	asd = v4l2_async_nf_add_fwnode_remote(&fmd->subdev_notifier,
					      of_fwnode_handle(ep),
					      struct v4l2_async_connection);

	of_node_put(ep);
    ...
}

Because the reference is already dropped in the helper function, wouldn't the
subsequent put by the iterator's loop advance or scope exit result in a
double-free and a use-after-free of the device node?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic