Re: [PATCH v2 2/8] clk: sunxi-ng: de2: Export register regmap for DE33

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Use-after-free and permanent probe failure due to registering a devm-managed regmap with syscon.
- [High] Unconditional probe failure if `CONFIG_MFD_SYSCON` is disabled.
--

commit ef866c925d5c9b0231c3dc8c7b9a9590e11b86ec
Author: Jernej Skrabec <jernej.skrabec@gmail.com>

clk: sunxi-ng: de2: Export register regmap for DE33

This commit adds a regmap for DE33 plane mapping registers and registers
it via syscon so the DRM driver can access them.

> diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-de2.c b/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
> --- a/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun8i-de2.c

[ ... ]

> @@ -336,13 +368,23 @@ static int sunxi_de2_clk_probe(struct platform_device *pdev)
>  	}
>  
>  	/*
> -	 * The DE33 requires these additional (unknown) registers set
> +	 * The DE33 requires these additional plane mapping registers set
>  	 * during initialisation.
>  	 */
>  	if (of_device_is_compatible(pdev->dev.of_node,
>  				    "allwinner,sun50i-h616-de33-clk")) {
> -		writel(0, reg + 0x24);
> -		writel(0x0000a980, reg + 0x28);
> +		struct regmap *regmap;
> +
> +		regmap = devm_regmap_init_mmio(&pdev->dev, reg,
> +					       &sun8i_de2_ccu_regmap_config);
> +		if (IS_ERR(regmap)) {
> +			ret = PTR_ERR(regmap);
> +			goto err_assert_reset;
> +		}
> +
> +		ret = of_syscon_register_regmap(dev_of_node(&pdev->dev), regmap);
> +		if (ret)
> +			goto err_assert_reset;

If syscon is disabled in the kernel configuration, the inline stub for
of_syscon_register_regmap() returns -EOPNOTSUPP. Will this unconditionally
fail the probe for the core display clock controller on systems without syscon
enabled?

Additionally, is it safe to register a devm-managed regmap with syscon?

of_syscon_register_regmap() adds the regmap to a global list but provides no
deregistration mechanism, as syscon assumes permanent mappings.

If the driver module is unbound by a user via sysfs, or if the subsequent
devm_sunxi_ccu_probe() call fails, devres will automatically free the regmap
and unmap the MMIO.

Would this leave a dangling pointer in the global syscon list, leading to a
use-after-free when the DRM driver looks it up? Also, wouldn't a subsequent
attempt to re-bind fail permanently with -EEXIST?

>  	}
>  
>  	ret = devm_sunxi_ccu_probe(&pdev->dev, reg, ccu_desc);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260509190015.79086-1-jernej.skrabec@siol.net?part=2