Re: [PATCH v7 1/5] clocksource/timer-econet-en751221: fix refcount leak

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Calculating num_blocks using num_possible_cpus() may lead to a NULL pointer dereference if the cpu_possible_mask has holes.
--

--- Patch [1]: [PATCH v7 1/5] clocksource/timer-econet-en751221: fix refcount leak ---
commit 7c522e295ecdd52460f4c1098da3b6a65821ce9c
Author: Alexander A. Klimov <grandmaster@al2klimov.de>

clocksource/timer-econet-en751221: fix refcount leak

Every value returned from of_clk_get() is supposed to be cleaned up
via clk_put() once not needed anymore.

> diff --git a/drivers/clocksource/timer-econet-en751221.c b/drivers/clocksource/timer-econet-en751221.c
> index 4008076b1a210..1859335345b59 100644
> --- a/drivers/clocksource/timer-econet-en751221.c
> +++ b/drivers/clocksource/timer-econet-en751221.c
> @@ -181,6 +181,7 @@ static int __init timer_init(struct device_node *np)
>  	}
>  
>  	econet_timer.freq_hz = clk_get_rate(clk);
> +	clk_put(clk);
>  
>  	for (int i = 0; i < num_blocks; i++) {

[Severity: High]
This is a pre-existing issue, but can this loop under-allocate memory blocks
and cause a NULL pointer dereference if the cpu_possible_mask has holes?

In timer_init(), num_blocks is calculated based on the count of possible CPUs:

    int num_blocks = DIV_ROUND_UP(num_possible_cpus(), 2);

If the CPU topology is sparse (for example, CPUs 0 and 2 are possible, but
not 1), this under-allocates memory blocks relative to the maximum CPU ID.
Only one block will be mapped, leaving econet_timer.membase[1] unmapped.

Later, cevt_init() iterates over the possible CPUs:

    for_each_possible_cpu(i) {
        ...
        cevt_dev_init(i);
    }

Which unconditionally accesses the unmapped memory:

    static void __init cevt_dev_init(uint cpu)
    {
        iowrite32(0, reg_count(cpu));
        ...

The reg_count() macro dereferences econet_timer.membase[cpu >> 1]. If a block
was left unmapped due to a gap in CPU IDs, membase[1] will be NULL. This causes
iowrite32() to dereference NULL + 0x08, resulting in a crash on boot.

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