Re: [PATCH v7 3/5] clocksource/timer-econet-en751221: Init teardown on error if possible

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] On systems with sparse CPU topologies, memory blocks mapped via num_possible_cpus() are out of bounds when indexed by absolute CPU ID.
--

--- Patch [3]: [PATCH v7 3/5] clocksource/timer-econet-en751221: Init teardown on error if possible ---
commit aa21baac1fd919824fefec289b6741bdd5585304
Author: Caleb James DeLisle <cjd@cjdns.fr>
clocksource/timer-econet-en751221: Init teardown on error if possible

The commit converts the init process of the econet timer into two phases,
moving IRQ and address resource mapping before the point of no return. It
adds teardown logic in case of an error before this point.

> diff --git a/drivers/clocksource/timer-econet-en751221.c b/drivers/clocksource/timer-econet-en751221.c
> index 1859335345b59..714702b9ef128 100644
> --- a/drivers/clocksource/timer-econet-en751221.c
> +++ b/drivers/clocksource/timer-econet-en751221.c
[ ... ]
> -static int __init cevt_init(struct device_node *np)
> +static void __init cevt_init(struct device_node *np)
>  {
[ ... ]
>  	for_each_possible_cpu(i) {
>  		struct clock_event_device *cd = &per_cpu(econet_timer_pcpu, i);
[ ... ]
>  		cevt_dev_init(i);

[Severity: High]
This isn't a bug introduced by this patch, but could this loop cause a NULL
pointer dereference on systems with sparse CPU topologies?

If a system has sparse CPU IDs (for example, CPU 0 and CPU 4),
num_possible_cpus() evaluates to 2. When timer_init() calculates num_blocks
using the popcount of possible CPUs, it maps only the first block into membase:

timer_init()
  int num_blocks = DIV_ROUND_UP(num_possible_cpus(), 2);
  ...
  for (i = 0; i < num_blocks; i++)
      econet_timer.membase[i] = of_iomap(np, i);

However, the loop here iterates using for_each_possible_cpu(i). For CPU ID 4,
cevt_dev_init(4) is called. The underlying register access in reg_count()
uses the absolute CPU ID to compute the array index:

reg_count()
  return econet_timer.membase[timer_n >> 1] + (timer_n & 1) * 0x08 + 0x08;

This calculates an index of 4 >> 1 = 2, accessing membase[2]. Because
num_blocks was calculated as 1, membase[2] was never mapped and remains NULL,
which would lead to a NULL pointer dereference during hardware initialization.

>  	}

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