network/stack: Fix a race condition in device interface teardown.

Inside the device_consumer thread, the sequence of operations was this:
  1. Call fifo_dequeue_buffer()
  2. If it returned B_INTERRUPTED, exit the thread. Otherwise, process
     the buffer it returned as normal.
  3. Loop.

Thus, if the FIFO was destroyed not during a call to
fifo_dequeue_buffer, the next loop, fifo_dequeue_buffer would
be called on a destroyed FIFO. It would then try to lock the mutex
in the FIFO which had been destroyed, causing an assertion failure
and thus a panic.

Now, we check the ref_count on every loop, and set it to 0 before
calling uninit_fifo(). Thus, even if we are in the middle of a loop
inside the FIFO thread, the loop iteration condition will fail
and thus the thread will exit, avoiding the race.

Probably this was not an issue before because the timing required
to hit this is incredibly unlikely. With the new ipro1000 driver
(or kallisti5's WIP TUN/TAP driver), the timing makes this much
more likely.

Should fix #15024.
This commit is contained in:
Augustin Cavalier
2019-04-22 18:41:25 -04:00
parent d117379205
commit f62a92ae05
@@ -88,7 +88,7 @@ device_consumer_thread(void* _interface)
net_device* device = interface->device;
net_buffer* buffer;
while (true) {
while (atomic_get(&interface->ref_count) > 0) {
ssize_t status = fifo_dequeue_buffer(&interface->receive_queue, 0,
B_INFINITE_TIMEOUT, &buffer);
if (status != B_OK) {
@@ -380,13 +380,16 @@ put_device_interface(struct net_device_interface* interface)
if (atomic_add(&interface->ref_count, -1) != 1)
return;
// Indicate we are in the process of destroying this interface
// by setting its ref_count to 0.
interface->ref_count = 0;
MutexLocker locker(sLock);
sInterfaces.Remove(interface);
locker.Unlock();
uninit_fifo(&interface->receive_queue);
status_t status;
wait_for_thread(interface->consumer_thread, &status);
wait_for_thread(interface->consumer_thread, NULL);
net_device* device = interface->device;
const char* moduleName = device->module->info.name;