cpu/stm32/periph/timer: prevent spurious IRQs

[Enoch247]

Contribution description

This patch hardens the STM32 timer driver against some possible causes of spurious IRQs.

I did not actually observe spurious IRQs happening, but this PR comes as the result of a deep dive into the timer code to solve the bug in #20924. This PR is a defensive measure only.

Testing procedure

Timers should continue to work.

Issues/PRs references

• sys/ztimer: fix re-scheduling of timers #20924
• Draft: sys/ztimer: guard against spurious ISR #20925

[benpicco]

I did not actually observe spurious IRQs happening

hmm I'm not convinced - what makes you think the current code is incorrect then?

[Enoch247]

what makes you think the current code is incorrect then?

The first change in timer_set() swaps the order of operations that already existed. Previously any pending IRQ was cleared, then the timer channel's new trigger value is set. Technically, it is possible that between those two operations, an IRQ could trigger if the timer reaches the old trigger value, before the new value is set.

The second change in timer_clear() adds the clearing of IRQ pending flag, whenever a timer's channel is cleared.

Both changes are intended to prevent situations like following (nonsense) example:

int *ptr = NULL;

void timer_callback(int channel)
{
    *ptr++;
}

void func(void)
{
    int val = 0;
    ptr = &val;

    timer_set(timer, 0, 100);

    // do some stuff

    const irq_state = irq_disable()
    timer_clear(timer, 0);  /* or */ timer_set(timer, 0, 999999);
    // at this point, callers will expect that the timer will not fire, or fire at some point way in the future
    ptr = NULL
    irq_restore(irq_state);
    // once we leave the critical section, spurious timer IRQ fires and NULL is dereferenced

   ptr = &val;
}

[benpicco]

Ok sounds reasonable.
While you're at it, can you also please add that whitespace after the while( on line 241 😉

[Enoch247]

While you're at it, can you also please add that whitespace after the while( on line 241 😉

Done.

[riot-ci]

Murdock results

✔️ PASSED

f24fc69 cpu/stm32/periph/timer: fix whitespace style

Success	Failures	Total	Runtime
10249	0	10249	15m:52s

Artifacts

• Documentation preview