Skip to content

Commit 7dcb79c

Browse files
committed
Fix heap corruption in timer subsystem
The system was encountering a critical kernel panic, manifesting as: *** KERNEL PANIC (-16370) – heap corruption or invalid free The core problem stemmed from a conflict between two memory management strategies for linked-list nodes. The timer module utilized a custom static memory pool for its nodes, while the generic list manipulation functions performed their own malloc() and free() operations. This architectural mismatch led to two critical bugs. Firstly, a double-free occurred in mo_timer_destroy(). The generic list_remove() function would free a list node's memory, and then the same function would attempt to return this already-freed node to the custom memory pool. Secondly, a use-after-free vulnerability existed in the _timer_tick_handler(). If an expired timer's callback function destroyed the timer, the handler would subsequently attempt to access the now-freed timer's memory to check its auto-reload status. Fix these issues by removing the custom memory pool and unifying all node memory management under the generic list API. The _timer_tick_handler() now also re-validates the timer's existence after its callback executes to prevent accessing freed memory.
1 parent 09435f8 commit 7dcb79c

File tree

1 file changed

+14
-3
lines changed

1 file changed

+14
-3
lines changed

kernel/timer.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,11 +227,14 @@ void _timer_tick_handler(void)
227227
/* Collect expired timers in one pass, limited to batch size */
228228
while (!list_is_empty(kcb->timer_list) &&
229229
expired_count < TIMER_BATCH_SIZE) {
230-
timer_t *t = (timer_t *) kcb->timer_list->head->next->data;
230+
list_node_t *node = kcb->timer_list->head->next;
231+
timer_t *t = (timer_t *) node->data;
231232

232233
if (now >= t->deadline_ticks) {
233234
expired_timers[expired_count++] = t;
234-
list_pop(kcb->timer_list); /* O(1) removal from head */
235+
kcb->timer_list->head->next = node->next;
236+
kcb->timer_list->length--;
237+
return_timer_node(node);
235238
} else {
236239
/* First timer not expired, so none further down are */
237240
break;
@@ -347,7 +350,15 @@ int32_t mo_timer_destroy(uint16_t id)
347350
}
348351

349352
/* Remove from master list */
350-
list_remove(all_timers_list, node);
353+
list_node_t *prev = all_timers_list->head;
354+
while (prev->next != all_timers_list->tail && prev->next != node)
355+
prev = prev->next;
356+
357+
if (likely(prev->next == node)) {
358+
prev->next = node->next;
359+
all_timers_list->length--;
360+
}
361+
351362
free(t);
352363
return_timer_node(node);
353364

0 commit comments

Comments
 (0)