[PATCH] clocksource/drivers/timer-imx-gpt: fix resource leak on init error paths

[PATCH] clocksource/drivers/timer-imx-gpt: fix resource leak on init error paths

[Haotian Zhang]

mxc_timer_init_dt() maps the timer registers with of_iomap() and obtains
the ipg and per clocks with of_clk_get_by_name(), but every error path
jumps to err_kfree, which only frees the imx_timer structure.  The
ioremap'd mapping and the clock references are therefore leaked when
irq_of_parse_and_map() or _mxc_timer_init() fails.

Unmap the I/O region and release the clocks on those error paths.

Fixes: 8051a993ce22 ("clocksource/drivers/timer-imx-gpt: Fix potential memory leak")
Assisted-by: DeepSeek-V4.1-Flash
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
---
 drivers/clocksource/timer-imx-gpt.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/timer-imx-gpt.c b/drivers/clocksource/timer-imx-gpt.c
index 8335bd7f8c6c..b8005054b30a 100644
--- a/drivers/clocksource/timer-imx-gpt.c
+++ b/drivers/clocksource/timer-imx-gpt.c
@@ -441,7 +441,7 @@ static int __init mxc_timer_init_dt(struct device_node *np,  enum imx_gpt_type t
 	imxtm->irq = irq_of_parse_and_map(np, 0);
 	if (imxtm->irq <= 0) {
 		ret = -EINVAL;
-		goto err_kfree;
+		goto err_unmap;
 	}
 
 	imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");
@@ -455,12 +455,19 @@ static int __init mxc_timer_init_dt(struct device_node *np,  enum imx_gpt_type t
 
 	ret = _mxc_timer_init(imxtm);
 	if (ret)
-		goto err_kfree;
+		goto err_clk_put;
 
 	initialized = 1;
 
 	return 0;
 
+err_clk_put:
+	if (!IS_ERR_OR_NULL(imxtm->clk_per))
+		clk_put(imxtm->clk_per);
+	if (!IS_ERR_OR_NULL(imxtm->clk_ipg))
+		clk_put(imxtm->clk_ipg);
+err_unmap:
+	iounmap(imxtm->base);
 err_kfree:
 	kfree(imxtm);
 	return ret;
-- 
2.25.1

Re: [PATCH] clocksource/drivers/timer-imx-gpt: fix resource leak on init error paths

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] clocksource: Use-After-Unmap of timer base by sched_clock on init error
- [Medium] clocksource: missing irq_dispose_mapping on init error path
--

commit e37f50884f58329bb1905901be3a80315d582d01
Author: Haotian Zhang <vulab@iscas.ac.cn>

clocksource/drivers/timer-imx-gpt: fix resource leak on init error paths

mxc_timer_init_dt() maps the timer registers with of_iomap() and obtains
the ipg and per clocks with of_clk_get_by_name(), but every error path
jumps to err_kfree, which only frees the imx_timer structure.  The
ioremap'd mapping and the clock references are therefore leaked when
irq_of_parse_and_map() or _mxc_timer_init() fails.

Unmap the I/O region and release the clocks on those error paths.

> diff --git a/drivers/clocksource/timer-imx-gpt.c b/drivers/clocksource/timer-imx-gpt.c
> index 8335bd7f8c6ce..b8005054b30ae 100644
> --- a/drivers/clocksource/timer-imx-gpt.c
> +++ b/drivers/clocksource/timer-imx-gpt.c
> @@ -441,7 +441,7 @@ static int __init mxc_timer_init_dt(struct device_node *np,  enum imx_gpt_type t
>  	imxtm->irq = irq_of_parse_and_map(np, 0);
>  	if (imxtm->irq <= 0) {
>  		ret = -EINVAL;
> -		goto err_kfree;
> +		goto err_unmap;
>  	}
>  
>  	imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");
> @@ -455,12 +455,19 @@ static int __init mxc_timer_init_dt(struct device_node *np,  enum imx_gpt_type t
>  
>  	ret = _mxc_timer_init(imxtm);
>  	if (ret)
> -		goto err_kfree;
> +		goto err_clk_put;
>  
>  	initialized = 1;
>  
>  	return 0;
>  
> +err_clk_put:
> +	if (!IS_ERR_OR_NULL(imxtm->clk_per))
> +		clk_put(imxtm->clk_per);
> +	if (!IS_ERR_OR_NULL(imxtm->clk_ipg))
> +		clk_put(imxtm->clk_ipg);

[Severity: Medium]
Does this error path in mxc_timer_init_dt() leak the IRQ mapping allocated
earlier by irq_of_parse_and_map()?

If _mxc_timer_init() fails, the code jumps here to clean up the clocks and
the ioremap, but it does not call irq_dispose_mapping(imxtm->irq) to clean
up the IRQ mapping.

> +err_unmap:
> +	iounmap(imxtm->base);

[Severity: High]
Can this unmap in mxc_timer_init_dt() cause a fatal use-after-unmap page
fault in the core kernel?

If _mxc_timer_init() fails late in its execution, it might return an error
after mxc_clocksource_init() has already run successfully. For example:

drivers/clocksource/timer-imx-gpt.c:_mxc_timer_init()
    ret = mxc_clocksource_init(imxtm);
    if (ret)
        return ret;
    return mxc_clockevent_init(imxtm);

If mxc_clockevent_init() returns an error, mxc_clocksource_init() has
already called sched_clock_register() and registered the clocksource using
imxtm->base. Since there are no APIs to unregister the sched_clock or
clocksource, they will continue to access this memory from hot paths like
timekeeping. Unmapping it here leaves those subsystems reading from freed
memory.

>  err_kfree:
>  	kfree(imxtm);
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008173604.2733975-1-vulab@iscas.ac.cn?part=1

Re: [PATCH] clocksource/drivers/timer-imx-gpt: fix resource leak on init error paths

[Frank Li]

On Thu, Oct 08, 2026 at 05:45:17PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] clocksource: Use-After-Unmap of timer base by sched_clock on init error
> - [Medium] clocksource: missing irq_dispose_mapping on init error path
> --

Look like this is real problem, please check it.

Frank

>
> commit e37f50884f58329bb1905901be3a80315d582d01
> Author: Haotian Zhang <vulab@iscas.ac.cn>
>
> clocksource/drivers/timer-imx-gpt: fix resource leak on init error paths
>
> mxc_timer_init_dt() maps the timer registers with of_iomap() and obtains
> the ipg and per clocks with of_clk_get_by_name(), but every error path
> jumps to err_kfree, which only frees the imx_timer structure.  The
> ioremap'd mapping and the clock references are therefore leaked when
> irq_of_parse_and_map() or _mxc_timer_init() fails.
>
> Unmap the I/O region and release the clocks on those error paths.
>
> > diff --git a/drivers/clocksource/timer-imx-gpt.c b/drivers/clocksource/timer-imx-gpt.c
> > index 8335bd7f8c6ce..b8005054b30ae 100644
> > --- a/drivers/clocksource/timer-imx-gpt.c
> > +++ b/drivers/clocksource/timer-imx-gpt.c
> > @@ -441,7 +441,7 @@ static int __init mxc_timer_init_dt(struct device_node *np,  enum imx_gpt_type t
> >  	imxtm->irq = irq_of_parse_and_map(np, 0);
> >  	if (imxtm->irq <= 0) {
> >  		ret = -EINVAL;
> > -		goto err_kfree;
> > +		goto err_unmap;
> >  	}
> >
> >  	imxtm->clk_ipg = of_clk_get_by_name(np, "ipg");
> > @@ -455,12 +455,19 @@ static int __init mxc_timer_init_dt(struct device_node *np,  enum imx_gpt_type t
> >
> >  	ret = _mxc_timer_init(imxtm);
> >  	if (ret)
> > -		goto err_kfree;
> > +		goto err_clk_put;
> >
> >  	initialized = 1;
> >
> >  	return 0;
> >
> > +err_clk_put:
> > +	if (!IS_ERR_OR_NULL(imxtm->clk_per))
> > +		clk_put(imxtm->clk_per);
> > +	if (!IS_ERR_OR_NULL(imxtm->clk_ipg))
> > +		clk_put(imxtm->clk_ipg);
>
> [Severity: Medium]
> Does this error path in mxc_timer_init_dt() leak the IRQ mapping allocated
> earlier by irq_of_parse_and_map()?
>
> If _mxc_timer_init() fails, the code jumps here to clean up the clocks and
> the ioremap, but it does not call irq_dispose_mapping(imxtm->irq) to clean
> up the IRQ mapping.
>
> > +err_unmap:
> > +	iounmap(imxtm->base);
>
> [Severity: High]
> Can this unmap in mxc_timer_init_dt() cause a fatal use-after-unmap page
> fault in the core kernel?
>
> If _mxc_timer_init() fails late in its execution, it might return an error
> after mxc_clocksource_init() has already run successfully. For example:
>
> drivers/clocksource/timer-imx-gpt.c:_mxc_timer_init()
>     ret = mxc_clocksource_init(imxtm);
>     if (ret)
>         return ret;
>     return mxc_clockevent_init(imxtm);
>
> If mxc_clockevent_init() returns an error, mxc_clocksource_init() has
> already called sched_clock_register() and registered the clocksource using
> imxtm->base. Since there are no APIs to unregister the sched_clock or
> clocksource, they will continue to access this memory from hot paths like
> timekeeping. Unmapping it here leaves those subsystems reading from freed
> memory.
>
> >  err_kfree:
> >  	kfree(imxtm);
> >  	return ret;
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261008173604.2733975-1-vulab@iscas.ac.cn?part=1