Re: [PATCH v11 4/5] clocksource/timer-econet-en751221: Support EN751627 without percpu IRQ

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] clocksource: memory leak of econet_timer.irqs on success path
--

commit ef0bb74d87af020e62a370267e55ff45f7da1e01
Author: Caleb James DeLisle <cjd@cjdns.fr>

    clocksource/timer-econet-en751221: Support EN751627 without percpu IRQ

    EN751627 is based on the 1004Kc which uses a different interrupt number
    for each CPU timer. Support both this and the EN751221 which uses a
    single percpu interrupt.

> diff --git a/drivers/clocksource/timer-econet-en751221.c b/drivers/clocksource/timer-econet-en751221.c
> index 1859335345b59..edb3db3125ce3 100644
> --- a/drivers/clocksource/timer-econet-en751221.c
> +++ b/drivers/clocksource/timer-econet-en751221.c
[ ... ]
>  static int __init timer_init(struct device_node *np)
>  {
> -	int num_blocks = DIV_ROUND_UP(num_possible_cpus(), 2);
> +	int num_blocks = of_address_count(np);
> +	int num_irqs = of_irq_count(np);
>  	struct clk *clk;
> -	int ret;
> +	int ret, i;
> +
[ ... ]
> +	econet_timer.num_irqs = num_irqs;
> +	econet_timer.irqs = kcalloc(num_irqs, sizeof(*econet_timer.irqs),
> +				    GFP_KERNEL);
> +	if (!econet_timer.irqs)
> +		return -ENOMEM;
> +
[ ... ]
>  	sched_clock_register(sched_clock_read, ECONET_BITS,
>  			     econet_timer.freq_hz);
> @@ -212,6 +291,32 @@ static int __init timer_init(struct device_node *np)
>  		(econet_timer.freq_hz / 1000) % 1000);
>  
>  	return 0;

[Severity: Low]
Does this code leak econet_timer.irqs on the success path?

The array is only used to populate the per-CPU cd->irq during initialization in
cevt_init() and does not appear to be accessed again. Could we safely call
kfree(econet_timer.irqs) here before returning 0, rather than retaining it
permanently?

> +
> +out_irq_free:
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015165404.519741-1-cjd@cjdns.fr?part=4