Skip to content

Commit

Permalink
Kernel: Fix UHCIController singleton startup null-deref race condition.
Browse files Browse the repository at this point in the history
The following KUBSAN crash on startup was reported on discord:

```
UHCI: Started
KUBSAN: reference binding to null pointer of type struct UHCIController
KUBSAN: at ../../Kernel/Devices/USB/UHCIController.cpp, line 67
```

After inspecting the code, it became clear that there's a window of time
where the kernel task which monitors the UHCI port can startup and start
executing before the UHCIController constructor completes. This leaves
the singleton pointing to nullptr, thus in the duration of this race
window the "UHCI port proc" thread will go an and de-reference the null
pointer when trying to read for status changes on the UHCI root ports.

Reported-by: @stelar7
Reported-by: @bcoles

Fixes: SerenityOS#6154
  • Loading branch information
bgianfo authored and awesomekling committed May 15, 2021
1 parent 3ba3d2d commit d76dedb
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions Kernel/Devices/USB/UHCIController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,10 @@ UNMAP_AFTER_INIT void UHCIController::detect()
return;

if (PCI::get_class(address) == 0xc && PCI::get_subclass(address) == 0x03 && PCI::get_programming_interface(address) == 0) {
if (!s_the)
if (!s_the) {
s_the = new UHCIController(address, id);
s_the->spawn_port_proc();
}
}
});
}
Expand All @@ -93,8 +95,6 @@ UNMAP_AFTER_INIT UHCIController::UHCIController(PCI::Address address, PCI::ID id

reset();
start();

spawn_port_proc();
}

UNMAP_AFTER_INIT UHCIController::~UHCIController()
Expand Down Expand Up @@ -134,7 +134,7 @@ void UHCIController::reset()

UNMAP_AFTER_INIT void UHCIController::create_structures()
{
// Let's allocate memory for botht the QH and TD pools
// Let's allocate memory for both the QH and TD pools
// First the QH pool and all of the Interrupt QH's
auto qh_pool_vmobject = ContiguousVMObject::create_with_size(2 * PAGE_SIZE);
m_qh_pool = MemoryManager::the().allocate_kernel_region_with_vmobject(*qh_pool_vmobject, 2 * PAGE_SIZE, "UHCI Queue Head Pool", Region::Access::Write);
Expand Down

0 comments on commit d76dedb

Please sign in to comment.