Aren/haiku
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

network stack: Fix deadlock in get_interface functions.

Browse Source 
If these methods are called while the interface in question
is receiving data via a receive thread, we can hit a deadlock
where a receive thread is holding the receive lock and then
tries to call get_interface_address_for_link (due, e.g., to ipv4
checking is_local_link_address), which tries to acquire the interfaces
lock, while at the same time we are trying to acquire the receive lock
due to CreateDomainDatalinkIfNeeded invoking a module's datalink_init
which calls register_device_handler, so we deadlock. (Whew!)

As far as I can tell, we do not need to set Busy() here despite unlocking
the interfaces lock, as the Interface acquires its own lock in
CreateDomainDatalinkIfNeeded.

Observed in VMware when the DHCP client spins for a long time,
and the deadlock occurs upon opening Network preferences.
This commit is contained in:
Augustin Cavalier 2022-03-10 14:27:40 -05:00
parent 2828ed2fa1
commit 488f8888a2
1 changed files with 24 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
src/add-ons/kernel/network/stack/interfaces.cpp
View File

@ -1426,11 +1426,16 @@ get_interface(net_domain* domain, uint32 index)
if (interface == NULL || interface->IsBusy())
return NULL;
// We must unlock before invoking CreateDomainDatalinkIfNeeded, because
// otherwise we can hit lock ordering inversions with receive threads,
// usually in register_device_handler.
BReference<Interface> interfaceRef(interface);
locker.Unlock();
if (interface->CreateDomainDatalinkIfNeeded(domain) != B_OK)
return NULL;
interface->AcquireReference();
return interface;
return interfaceRef.Detach();
}
@ -1443,11 +1448,14 @@ get_interface(net_domain* domain, const char* name)
if (interface == NULL || interface->IsBusy())
return NULL;
// See comment in get_interface.
BReference<Interface> interfaceRef(interface);
locker.Unlock();
if (interface->CreateDomainDatalinkIfNeeded(domain) != B_OK)
return NULL;
interface->AcquireReference();
return interface;
return interfaceRef.Detach();
}
@ -1461,11 +1469,15 @@ get_interface_for_device(net_domain* domain, uint32 index)
if (interface->device->index == index) {
if (interface->IsBusy())
return NULL;
// See comment in get_interface.
BReference<Interface> interfaceRef(interface);
locker.Unlock();
if (interface->CreateDomainDatalinkIfNeeded(domain) != B_OK)
return NULL;
interface->AcquireReference();
return interface;
return interfaceRef.Detach();
}
}
@ -1499,11 +1511,15 @@ get_interface_for_link(net_domain* domain, const sockaddr* _linkAddress)
&& linkAddress.sdl_index == interface->index)) {
if (interface->IsBusy())
return NULL;
// See comment in get_interface.
BReference<Interface> interfaceRef(interface);
locker.Unlock();
if (interface->CreateDomainDatalinkIfNeeded(domain) != B_OK)
return NULL;
interface->AcquireReference();
return interface;
return interfaceRef.Detach();
}
}