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

[Frank Li <Frank.li@oss.nxp.com>]

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