eal: fix alarm cancel list walk

PATCH In review Stephen Hemminger 1 participant 2 messages Sep 4

Share & tools

Stephen Hemminger

author

2026-09-04 20:20 UTC #0 lore raw

PATCH 1/1 eal: fix alarm cancel list walk

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 [email protected]

Signed-off-by Stephen Hemminger <[email protected]>

---
 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(-)

lib/eal/freebsd/eal_alarm.c +7 ~~-34~~

index c03e281e67..1585a651e9 100644
@@ -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);

lib/eal/linux/eal_alarm.c +13 ~~-27~~

index a1433eb867..eb41064851 100644
@@ -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);

lib/eal/windows/eal_alarm.c +2 -2

index 0b11d331dc..ed6e7f2245 100644
@@ -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;

Signature · 1 line

2.53.0

Stephen Hemminger

author

2026-09-04 20:46 UTC #1 lore raw

Quoted 24 lines · On Fri, 4 Sep 2026 13:20:31 -0700 Stephen Hemminger…

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.

Quoted 6 lines

Fixes af75078fece3 ("first public release")
Fixes f4cbdbc7fbd2 ("eal/windows: implement alarm API")
Cc [email protected]
Signed-off-by Stephen Hemminger <[email protected]>
---

The windows bug is real, the other platforms it is not a bug but just Gcc getting confused. Will split and resend