drm/sun4i: fix refcount leak in sun4i_backend_init_sat()

When sun4i_backend_init_sat() calls reset_control_deassert() it
increments the deassert_count of the reset controller, and must
pair that with a reset_control_assert() call to decrement it.
In the error path where clk_prepare_enable() fails, the function
returns immediately without calling reset_control_assert(), leaking
the reference count.  Other error paths, like the devm_clk_get()
failure, correctly jump to the err_assert_reset label which performs
the missing assert.

Fix the leak by using the existing err_assert_reset label in the
clk_prepare_enable error path instead of returning directly.

Cc: stable@vger.kernel.org
Fixes: 440d2c7b127a ("drm/sun4i: backend: Handle the SAT")
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

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

Pre-existing issues:
- [High] Missing cleanup for SAT resources in sun4i_backend_bind() if devm_regmap_init_mmio() fails, leaking SAT clock and reset state.
- [High] Return values of clk_prepare_enable() are completely ignored, potentially causing imprecise external aborts on clock enablement failure.
- [High] Use of devm_ managed allocations inside the component bind() callback causes resource leaks on every rebind.
--

commit bc39d4e776a1c47107cfd91202ce280e5f321f36
Author: Wentao Liang <vulab@iscas.ac.cn>

drm/sun4i: fix refcount leak in sun4i_backend_init_sat()

When sun4i_backend_init_sat() calls reset_control_deassert() it
increments the deassert_count of the reset controller, and must
pair that with a reset_control_assert() call to decrement it.
In the error path where clk_prepare_enable() fails, the function
returns immediately without calling reset_control_assert(), leaking
the reference count.  Other error paths, like the devm_clk_get()
failure, correctly jump to the err_assert_reset label which performs
the missing assert.

Fix the leak by using the existing err_assert_reset label in the
clk_prepare_enable error path instead of returning directly.

> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index 03e46a2af6463..adf5b6e09ca64 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -686,7 +686,7 @@ static int sun4i_backend_init_sat(struct device *dev) {
>  	ret = clk_prepare_enable(backend->sat_clk);
>  	if (ret) {
>  		dev_err(dev, "Couldn't enable the SAT clock\n");
> -		return ret;
> +		goto err_assert_reset;
>  	}
>  
>  	return 0;

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing issue
where SAT resources are not cleaned up in sun4i_backend_bind() if
devm_regmap_init_mmio() fails.

If the device is compatible with sun8i-a33-display-backend,
sun4i_backend_init_sat() successfully enables sat_clk and deasserts sat_reset.

If the subsequent call to devm_regmap_init_mmio() fails:

drivers/gpu/drm/sun4i/sun4i_backend.c:sun4i_backend_bind() {
    ...
    backend->engine.regs = devm_regmap_init_mmio(dev, regs,
                             &sun4i_backend_regmap_config);
    if (IS_ERR(backend->engine.regs)) {
        dev_err(dev, "Couldn't create the backend regmap\n");
        ret = PTR_ERR(backend->engine.regs);
        goto err_disable_ram_clk;
    }
    ...
}

Will jumping to err_disable_ram_clk skip calling sun4i_backend_free_sat() or
otherwise clean up the SAT resources, permanently leaving the SAT clock enabled
and the reset deasserted?

[Severity: High]
This is another pre-existing issue not introduced by this patch, but the
return values of clk_prepare_enable() are ignored in sun4i_backend_bind(),
which might cause imprecise external aborts.

drivers/gpu/drm/sun4i/sun4i_backend.c:sun4i_backend_bind() {
    ...
    clk_prepare_enable(backend->bus_clk);
    ...
    clk_prepare_enable(backend->mod_clk);
    ...
    clk_prepare_enable(backend->ram_clk);
    ...
}

If one of these clocks fails to enable, could the driver proceed to access
hardware registers via regmap on an unclocked hardware block, leading to
system hangs or crashes?

[Severity: High]
This is also a pre-existing issue not introduced by this patch, but using devm_
managed allocations inside the component bind() callback can cause resource
leaks on every rebind.

drivers/gpu/drm/sun4i/sun4i_backend.c:sun4i_backend_bind() {
    ...
    backend = devm_kzalloc(dev, sizeof(*backend), GFP_KERNEL);
    ...
}

Since a component's bind/unbind lifecycle can occur multiple times without the
underlying platform device being removed (for instance, when the master DRM
device is manually unbound via sysfs), are these managed resources prevented
from being freed upon unbind()?

Can this cause a persistent memory and resource leak (including clocks,
regmaps, and reset controls) on every unbind/rebind cycle?
  

Le 07/06/2026 à 05:09, Wentao Liang a écrit :
> When sun4i_backend_init_sat() calls reset_control_deassert() it
> increments the deassert_count of the reset controller, and must
> pair that with a reset_control_assert() call to decrement it.
> In the error path where clk_prepare_enable() fails, the function
> returns immediately without calling reset_control_assert(), leaking
> the reference count.  Other error paths, like the devm_clk_get()
> failure, correctly jump to the err_assert_reset label which performs
> the missing assert.
> 
> Fix the leak by using the existing err_assert_reset label in the
> clk_prepare_enable error path instead of returning directly.
> 
> Cc: stable@vger.kernel.org
> Fixes: 440d2c7b127a ("drm/sun4i: backend: Handle the SAT")
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>   drivers/gpu/drm/sun4i/sun4i_backend.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index bc35dad53b07..c9ec5fc26f7e 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -686,7 +686,7 @@ static int sun4i_backend_init_sat(struct device *dev) {
>   	ret = clk_prepare_enable(backend->sat_clk);
>   	if (ret) {
>   		dev_err(dev, "Couldn't enable the SAT clock\n");
> -		return ret;
> +		goto err_assert_reset;
>   	}
>   
>   	return 0;

Hi,

another way to fix it and simplify the code at the same time would be to 
use devm_reset_control_get_exclusive_deasserted() and 
devm_clk_get_enabled().

just my 2c,

CJ
  

Dne nedelja, 7. junij 2026 ob 05:09:50 Srednjeevropski poletni čas je Wentao Liang napisal(a):
> When sun4i_backend_init_sat() calls reset_control_deassert() it
> increments the deassert_count of the reset controller, and must
> pair that with a reset_control_assert() call to decrement it.
> In the error path where clk_prepare_enable() fails, the function
> returns immediately without calling reset_control_assert(), leaking
> the reference count.  Other error paths, like the devm_clk_get()
> failure, correctly jump to the err_assert_reset label which performs
> the missing assert.
> 
> Fix the leak by using the existing err_assert_reset label in the
> clk_prepare_enable error path instead of returning directly.
> 
> Cc: stable@vger.kernel.org
> Fixes: 440d2c7b127a ("drm/sun4i: backend: Handle the SAT")
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej