Re: [PATCH v6 2/4] clocksource/drivers: Add support for the Renesas RZ/N1 timers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] timer: rzn1: NULL pointer deref in irq handler for detached timers
--

commit c504b8f1cef80ec82616cd19b137210a244f7ec4
Author: Herve Codina (Schneider Electric) <herve.codina@bootlin.com>

clocksource/drivers: Add support for the Renesas RZ/N1 timers

The Renesas RZ/N1 timer block controller is the controller in charge of
timers available in the Renesas RZ/N1 SoCs family.

> diff --git a/drivers/clocksource/timer-rzn1.c b/drivers/clocksource/timer-rzn1.c
> new file mode 100644
> index 0000000000000..3b173b4086e92
> --- /dev/null
> +++ b/drivers/clocksource/timer-rzn1.c

[ ... ]

> +static irqreturn_t rzn1_timer_interrupt(int irq, void *dev_id)
> +{
> +	struct rzn1_timer *timer = dev_id;
> +
> +	/* Ack the interrupt */
> +	readl(timer->base + RZN1_TIMER_CLEAR_INT_REG);
> +
> +	timer->ced.event_handler(&timer->ced);

[Severity: High]
Does this code risk a NULL pointer dereference if the timer is detached?

The driver unconditionally registers multiple clockevent devices and their IRQs.
The clockevents core leaves unused timers in CLOCK_EVT_STATE_DETACHED, which
means their event_handler remains NULL.

> +	return IRQ_HANDLED;
> +}

[ ... ]

> +static void rzn1_timer_register_clkevt(struct rzn1_timer *tab_timers,
> +				       unsigned int first_timer_index,
> +				       unsigned int last_timer_index)
> +{
> +	struct rzn1_timer *timer;
> +	unsigned int i;
> +
> +	for (i = first_timer_index; i <= last_timer_index; i++) {
> +		timer = &tab_timers[i];
> +		clockevents_config_and_register(&timer->ced, timer->rate,
> +						1, GENMASK(timer->width - 1, 0));
> +		/*
> +		 * Time to enable the interrupt even if no interrupt can be
> +		 * fired yet. Indeed, the timer is still globally disabled.
> +		 * It will be enabled when rzn1_timer_config() is called
> +		 */
> +		enable_irq(timer->ced.irq);

[Severity: High]
Can enabling the IRQ unconditionally here trigger the NULL pointer dereference
above? 

If the clockevents core leaves a timer unused and we enable its IRQ anyway,
a spurious interrupt on this hardware line will cause the interrupt handler to
blindly call timer->ced.event_handler and crash.

> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929085035.57609-1-herve.codina@bootlin.com?part=2