Skip to content

Commit 6013202

Browse files
committed
lltable: Fix use-after-free race in llentry_free()
When callout_stop() returns 0, the timer callback is currently executing on another CPU. The original code proceeded to free the llentry anyway, causing a use-after-free when the timer callback (e.g., arptimer) accessed the freed memory. Fix by checking the callout_stop() return value: - If >0: timer was pending and successfully cancelled, drop timer's ref - If 0 and refcnt>1: another thread racing with timer, drop ref and bail; timer's llentry_free() will free the entry - If 0 and refcnt==1: we ARE the timer, proceed to free - If <0: timer was not scheduled, proceed normally PR: 285813 Signed-off-by: Teddy Engel <engel.teddy@gmail.com>
1 parent 8dad295 commit 6013202

1 file changed

Lines changed: 24 additions & 2 deletions

File tree

sys/net/if_llatbl.c

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -649,16 +649,38 @@ size_t
649649
llentry_free(struct llentry *lle)
650650
{
651651
size_t pkts_dropped;
652+
int ret;
652653

653654
LLE_WLOCK_ASSERT(lle);
654655

655656
KASSERT((lle->la_flags & LLE_LINKED) == 0, ("freeing linked lle"));
656657

657658
pkts_dropped = lltable_drop_entry_queue(lle);
658659

659-
/* cancel timer */
660-
if (callout_stop(&lle->lle_timer) > 0)
660+
/*
661+
* Cancel timer. Handle races with timer callback.
662+
*/
663+
ret = callout_stop(&lle->lle_timer);
664+
if (ret > 0) {
661665
LLE_REMREF(lle);
666+
} else if (ret == 0) {
667+
/*
668+
* Timer callback is executing. Two cases:
669+
*
670+
* refcnt > 1: Another thread is calling us while the timer
671+
* is running. The entry is already unlinked (KASSERT above),
672+
* so timer will skip its LLE_REMREF. Drop one ref and bail;
673+
* timer's llentry_free() will free the entry.
674+
*
675+
* refcnt == 1: We are the timer callback and already dropped
676+
* our ref before calling llentry_free(). Proceed to free.
677+
*/
678+
if (lle->lle_refcnt > 1) {
679+
LLE_REMREF(lle);
680+
LLE_WUNLOCK(lle);
681+
return (pkts_dropped);
682+
}
683+
}
662684
LLE_FREE_LOCKED(lle);
663685

664686
return (pkts_dropped);

0 commit comments

Comments
 (0)