Team2890/allwpilib
4
0
Fork 0
mirror of https://github.com/wpilibsuite/allwpilib synced 2026-06-24 01:31:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

HAL Notifier: Use freelist to avoid use-after-free race. (#50)

Browse Source 
As the notifierMutex is unlocked while the callback is being called in
alarmCallback(), it's possible for next to be invalidated due to a
cleanNotifier() call.  Now, instead of deleting the notifier immediately,
add it to a freelist to be cleaned up at the tail end of alarmCallback.
This commit is contained in:
Peter Johnson
2016-05-22 23:21:45 -07:00
parent 8fc55c80a9
commit 21b1e39b2a
1 changed files with 25 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
hal/lib/athena/Notifier.cpp
View File

@@ -21,12 +21,14 @@ static tAlarm* notifierAlarm = nullptr;
static tInterruptManager* notifierManager = nullptr;
static uint64_t closestTrigger = UINT64_MAX;
struct Notifier {
Notifier *prev, *next;
Notifier *prev, *next, *freeNext;
bool freed;
void* param;
void (*process)(uint64_t, void*);
uint64_t triggerTime = UINT64_MAX;
};
static Notifier* notifiers = nullptr;
static Notifier* notifiersFreeList = nullptr;
static std::atomic_flag notifierAtexitRegistered = ATOMIC_FLAG_INIT;
static std::atomic_int notifierRefCount{0};
@@ -42,7 +44,7 @@ static void alarmCallback(uint32_t, void*) {
// process all notifiers
Notifier* notifier = notifiers;
while (notifier) {
if (notifier->triggerTime != UINT64_MAX) {
if (!notifier->freed && notifier->triggerTime != UINT64_MAX) {
if (currentTime == 0) currentTime = getFPGATime(&status);
if (notifier->triggerTime < currentTime) {
notifier->triggerTime = UINT64_MAX;
@@ -57,6 +59,15 @@ static void alarmCallback(uint32_t, void*) {
}
notifier = notifier->next;
}
// clean up freelist
notifier = notifiersFreeList;
while (notifier) {
Notifier* next = notifier->freeNext;
delete notifier;
notifier = next;
}
notifiersFreeList = nullptr;
}
static void cleanupNotifierAtExit() {
@@ -91,7 +102,9 @@ void* initializeNotifier(void (*process)(uint64_t, void*), void* param,
Notifier* notifier = new Notifier();
notifier->prev = nullptr;
notifier->next = notifiers;
notifier->freeNext = nullptr;
if (notifier->next) notifier->next->prev = notifier;
notifier->freed = false;
notifier->param = param;
notifier->process = process;
notifiers = notifier;
@@ -103,11 +116,18 @@ void cleanNotifier(void* notifier_pointer, int32_t* status) {
std::lock_guard<priority_recursive_mutex> sync(notifierMutex);
Notifier* notifier = (Notifier*)notifier_pointer;
// remove from list and delete
// ignore double-free
if (notifier->freed) return;
notifier->freed = true;
// remove from list
if (notifier->prev) notifier->prev->next = notifier->next;
if (notifier->next) notifier->next->prev = notifier->prev;
if (notifiers == notifier) notifiers = notifier->next;
delete notifier;
// add to freelist
notifier->freeNext = notifiersFreeList;
notifiersFreeList = notifier;
}
if (notifierRefCount.fetch_sub(1) == 1) {
@@ -136,6 +156,7 @@ void updateNotifierAlarm(void* notifier_pointer, uint64_t triggerTime,
std::lock_guard<priority_recursive_mutex> sync(notifierMutex);
Notifier* notifier = (Notifier*)notifier_pointer;
if (notifier->freed) return;
notifier->triggerTime = triggerTime;
bool wasActive = (closestTrigger != UINT64_MAX);