furi/core/timer: resolve timer handle use-after-free post deletion (#3431)

When xTimerDelete is called using a dymanic timer handle, the timer
handle should immediately be considered unusable for any operation;
including checking if the timer is still running. Under high system
loads, that memory region may see fast reuse while furi_timer_free
is sleeping between timer active checks. That reuse could result in
memory at that pointer causing the timer active check to return true.

Rework the furi_timer_delete process (in the case of dynamically
allocated callback memory) to stop the timer, wait for it to stop,
free the memory, and then delete the timer. Timers without dynamically
allocated callback memory are just sent a delete command; no need
to stop it first.

Fixes: ff33bc6aea ("Furi: wait for timer wind down in destructor (#1716)")

Signed-off-by: Kris Bahnsen <Kris@KBEmbedded.com>
This commit is contained in:
Kris Bahnsen 2024-02-08 01:22:03 -08:00 committed by GitHub
parent afd41aade1
commit 26da5f564b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -67,17 +67,22 @@ void furi_timer_free(FuriTimer* instance) {
callb = (TimerCallback_t*)pvTimerGetTimerID(hTimer);
furi_check(xTimerDelete(hTimer, portMAX_DELAY) == pdPASS);
while(furi_timer_is_running(instance)) furi_delay_tick(2);
if((uint32_t)callb & 1U) {
/* If callback memory was allocated, it is only safe to free it with
* the timer inactive. Send a stop command and wait for the timer to
* be in an inactive state.
*/
furi_check(xTimerStop(hTimer, portMAX_DELAY) == pdPASS);
while(furi_timer_is_running(instance)) furi_delay_tick(2);
/* Callback memory was allocated from dynamic pool, clear flag */
callb = (TimerCallback_t*)((uint32_t)callb & ~1U);
/* Return allocated memory to dynamic pool */
free(callb);
}
furi_check(xTimerDelete(hTimer, portMAX_DELAY) == pdPASS);
}
FuriStatus furi_timer_start(FuriTimer* instance, uint32_t ticks) {
@ -170,4 +175,4 @@ void furi_timer_set_thread_priority(FuriTimerThreadPriority priority) {
} else {
furi_crash();
}
}
}