Fix memory corruption when work queue is being deleted

When the last WorkItem is deleted, it is removed from a work queue and the
queue is being stopped. But, the queue itself might get deleted in the middle,
in a higher priority thread than where the WorkItem deletion was performed from

If the WorkQueue::Detach accesses the member variables after this, there is memory
corruption

This happens in particular when launching i2c or spi devices in
I2CSPIDriverBase::module_start:

- The "initializer" is deleted when the instance is not found and the iterator
  while loop continues.
- The workqueue is deleted in the middle of "initializer" deletion when the
  WorkQueueRunner returns.

This prevents deletion of the WorkQueue before the Detach has been finished,
in the specific case that the ::Detach triggers the deletion

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
This commit is contained in:
Jukka Laitinen 2021-08-24 12:54:49 +03:00 committed by Daniel Agar
parent ca744626cd
commit d1c09ec358
No known key found for this signature in database
GPG Key ID: FD3CBA98017A69DE
2 changed files with 23 additions and 0 deletions

View File

@ -96,6 +96,7 @@ private:
IntrusiveQueue<WorkItem *> _q;
px4_sem_t _process_lock;
px4_sem_t _exit_lock;
const wq_config_t &_config;
BlockingList<WorkItem *> _work_items;
px4::atomic_bool _should_exit{false};

View File

@ -59,11 +59,20 @@ WorkQueue::WorkQueue(const wq_config_t &config) :
px4_sem_init(&_process_lock, 0, 0);
px4_sem_setprotocol(&_process_lock, SEM_PRIO_NONE);
px4_sem_init(&_exit_lock, 0, 1);
px4_sem_setprotocol(&_exit_lock, SEM_PRIO_NONE);
}
WorkQueue::~WorkQueue()
{
work_lock();
// Synchronize with ::Detach
px4_sem_wait(&_exit_lock);
px4_sem_destroy(&_exit_lock);
px4_sem_destroy(&_process_lock);
work_unlock();
@ -83,11 +92,14 @@ bool WorkQueue::Attach(WorkItem *item)
}
work_unlock();
return false;
}
void WorkQueue::Detach(WorkItem *item)
{
bool exiting = false;
work_lock();
_work_items.remove(item);
@ -96,11 +108,21 @@ void WorkQueue::Detach(WorkItem *item)
// shutdown, no active WorkItems
PX4_DEBUG("stopping: %s, last active WorkItem closing", _config.name);
// Deletion of this work queue might happen right after request_stop or
// SignalWorkerThread. Use a separate lock to prevent premature deletion
px4_sem_wait(&_exit_lock);
exiting = true;
request_stop();
SignalWorkerThread();
}
work_unlock();
// In case someone is deleting this wq already, signal
// that it is now allowed
if (exiting) {
px4_sem_post(&_exit_lock);
}
}
void WorkQueue::Add(WorkItem *item)