Re: [PATCH 2/2] drm/tegra: Make sure clk_init_data is fully initialized

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing bounds check on the `nvidia,interface` device tree property causes out-of-bounds array indexing and arbitrary MMIO access.
--

--- Patch [2]: [PATCH 2/2] drm/tegra: Make sure clk_init_data is fully initialized ---
commit d6e18a3c09cc145658765914a32b191ad5da6aa8
Author: Geert Uytterhoeven <geert+renesas@glider.be>

drm/tegra: Make sure clk_init_data is fully initialized

The clk_init_data structure contains several mutually-exclusive members
for different methods to specify the possible parents of a clock,
prompting drivers to initialize only the members they need.  However,
not initializing all members may cause subtle issues, which are only
exposed when CONFIG_INIT_STACK_ALL_PATTERN or CONFIG_INIT_STACK_NONE is
enabled. Make sure all members are fully initialized, to avoid such bugs,
and to prevent future breakage when converting drivers to a different
method for specifying the parents.

> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index 802d2db7007af..c28557d0c96e5 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -596,8 +596,8 @@ static const struct clk_ops tegra_clk_sor_pad_ops = {
>  static struct clk *tegra_clk_sor_pad_register(struct tegra_sor *sor,
>  					      const char *name)
>  {
> +	struct clk_init_data init = {};
>  	struct tegra_clk_sor_pad *pad;
> -	struct clk_init_data init;
>  	struct clk *clk;
>  
>  	pad = devm_kzalloc(sor->dev, sizeof(*pad), GFP_KERNEL);

[ ... ]

>  	pad->sor = sor;
>  
>  	init.name = name;
>  	init.flags = 0;
>  	init.parent_names = tegra_clk_sor_pad_parents[sor->index];

[Severity: High]
This is a pre-existing issue, but can this out-of-bounds array indexing
occur if the Device Tree provides an invalid "nvidia,interface" value?

In tegra_sor_parse_dt(), the "nvidia,interface" property is read and
assigned directly to sor->index without any bounds checking:

drivers/gpu/drm/tegra/sor.c:tegra_sor_parse_dt() {
	...
	err = of_property_read_u32(np, "nvidia,interface", &value);
	if (err < 0)
		return err;

	sor->index = value;
	...
}

If a malformed Device Tree is provided, does using this unbounded sor->index to
access the tegra_clk_sor_pad_parents array lead to an out-of-bounds memory read?

It appears it could also cause arbitrary MMIO reads and writes when computing
register offsets using DC_DISP_CORE_SOR_SET_CONTROL(sor->index).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787239944.git.geert+renesas@glider.be?part=2