[PATCH] eal: fix alarm cancel list walk
From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2026-09-04 20:20:45
Subsystem:
library code, the rest · Maintainers:
Andrew Morton, Linus Torvalds
All three implementations of rte_eal_alarm_cancel() free entries while
walking the alarm list with LIST_FOREACH, which leaves the iterator
pointing into freed memory.
Linux and FreeBSD use two loops: one draining matches from the head of
the list, then a LIST_FOREACH over the rest that frees the current
entry and assigns the saved ap_prev to ap so iteration resumes from the
predecessor. ap_prev is only refreshed to a live entry by an iteration
that does not remove, and the head loop leaves it NULL when it empties
the list. A removal in the second loop then sets ap to NULL or to an
already freed entry, and the LIST_FOREACH increment dereferences it.
GCC -fanalyzer reports the freed case:
lib/eal/linux/eal_alarm.c:224:44: warning: use after 'free' of 'ap'
[CWE-416] [-Wanalyzer-use-after-free]
Windows has no such dance: it calls alarm_remove_unsafe() straight from
the loop body, so the increment reads freed memory on every removal but
the last.
Replace all of these with LIST_FOREACH_SAFE. FreeBSD sys/queue.h and
the bundled Windows sys/queue.h already provide it; glibc does not, so
define it locally as is already done in several drivers.
Fixes: af75078fece3 ("first public release")
Fixes: f4cbdbc7fbd2 ("eal/windows: implement alarm API")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/eal/freebsd/eal_alarm.c | 41 +++++++------------------------------
lib/eal/linux/eal_alarm.c | 40 ++++++++++++------------------------
lib/eal/windows/eal_alarm.c | 4 ++--
3 files changed, 22 insertions(+), 63 deletions(-)
diff --git a/lib/eal/freebsd/eal_alarm.c b/lib/eal/freebsd/eal_alarm.c
index c03e281e67..1585a651e9 100644
--- a/lib/eal/freebsd/eal_alarm.c
+++ b/lib/eal/freebsd/eal_alarm.c@@ -264,7 +264,7 @@ RTE_EXPORT_SYMBOL(rte_eal_alarm_cancel) int rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void *cb_arg) { - struct alarm_entry *ap, *ap_prev; + struct alarm_entry *ap, *ap_next; int count = 0; int err = 0; int executing;
@@ -277,15 +277,12 @@ rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void *cb_arg) do { executing = 0; rte_spinlock_lock(&alarm_list_lk); - /* remove any matches at the start of the list */ - while (1) { - ap = LIST_FIRST(&alarm_list); - if (ap == NULL) - break; - if (cb_fn != ap->cb_fn) - break; - if (cb_arg != ap->cb_arg && cb_arg != (void *) -1) - break; + + LIST_FOREACH_SAFE(ap, &alarm_list, next, ap_next) { + if (cb_fn != ap->cb_fn || + (cb_arg != (void *)-1 && cb_arg != ap->cb_arg)) + continue; + if (ap->executing == 0) { LIST_REMOVE(ap, next); free(ap);
@@ -301,31 +298,7 @@ rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void *cb_arg) executing++; else err = EINPROGRESS; - - break; - } - } - ap_prev = ap; - - /* now go through list, removing entries not at start */ - LIST_FOREACH(ap, &alarm_list, next) { - /* this won't be true first time through */ - if (cb_fn == ap->cb_fn && - (cb_arg == (void *)-1 || - cb_arg == ap->cb_arg)) { - if (ap->executing == 0) { - LIST_REMOVE(ap, next); - free(ap); - count++; - ap = ap_prev; - } else if (pthread_equal(ap->executing_id, - pthread_self()) == 0) { - executing++; - } else { - err = EINPROGRESS; - } } - ap_prev = ap; } rte_spinlock_unlock(&alarm_list_lk);
diff --git a/lib/eal/linux/eal_alarm.c b/lib/eal/linux/eal_alarm.c
index a1433eb867..eb41064851 100644
--- a/lib/eal/linux/eal_alarm.c
+++ b/lib/eal/linux/eal_alarm.c@@ -25,6 +25,13 @@ #define TFD_NONBLOCK O_NONBLOCK #endif +#ifndef LIST_FOREACH_SAFE +#define LIST_FOREACH_SAFE(var, head, field, tvar) \ + for ((var) = LIST_FIRST((head)); \ + (var) && ((tvar) = LIST_NEXT((var), field), 1); \ + (var) = (tvar)) +#endif + #define NS_PER_US 1000 #define US_PER_MS 1000 #define MS_PER_S 1000
@@ -206,7 +213,7 @@ RTE_EXPORT_SYMBOL(rte_eal_alarm_cancel) int rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void *cb_arg) { - struct alarm_entry *ap, *ap_prev; + struct alarm_entry *ap, *ap_next; int count = 0; int err = 0; int executing;
@@ -219,10 +226,11 @@ rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void *cb_arg) do { executing = 0; rte_spinlock_lock(&alarm_list_lk); - /* remove any matches at the start of the list */ - while ((ap = LIST_FIRST(&alarm_list)) != NULL && - cb_fn == ap->cb_fn && - (cb_arg == (void *)-1 || cb_arg == ap->cb_arg)) { + + LIST_FOREACH_SAFE(ap, &alarm_list, next, ap_next) { + if (cb_fn != ap->cb_fn || + (cb_arg != (void *)-1 && cb_arg != ap->cb_arg)) + continue; if (ap->executing == 0) { LIST_REMOVE(ap, next);
@@ -236,29 +244,7 @@ rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void *cb_arg) executing++; else err = EINPROGRESS; - - break; - } - } - ap_prev = ap; - - /* now go through list, removing entries not at start */ - LIST_FOREACH(ap, &alarm_list, next) { - /* this won't be true first time through */ - if (cb_fn == ap->cb_fn && - (cb_arg == (void *)-1 || cb_arg == ap->cb_arg)) { - - if (ap->executing == 0) { - LIST_REMOVE(ap, next); - free(ap); - count++; - ap = ap_prev; - } else if (pthread_equal(ap->executing_id, pthread_self()) == 0) - executing++; - else - err = EINPROGRESS; } - ap_prev = ap; } rte_spinlock_unlock(&alarm_list_lk);
diff --git a/lib/eal/windows/eal_alarm.c b/lib/eal/windows/eal_alarm.c
index 0b11d331dc..ed6e7f2245 100644
--- a/lib/eal/windows/eal_alarm.c
+++ b/lib/eal/windows/eal_alarm.c@@ -190,7 +190,7 @@ RTE_EXPORT_SYMBOL(rte_eal_alarm_cancel) int rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void *cb_arg) { - struct alarm_entry *ap; + struct alarm_entry *ap, *ap_next; unsigned int state; int removed; bool executing;
@@ -207,7 +207,7 @@ rte_eal_alarm_cancel(rte_eal_alarm_callback cb_fn, void *cb_arg) rte_spinlock_lock(&alarm_lock); - LIST_FOREACH(ap, &alarm_list, next) { + LIST_FOREACH_SAFE(ap, &alarm_list, next, ap_next) { if (!alarm_matches(ap, cb_fn, cb_arg)) continue;
--
2.53.0