From 6be5438599859793010129d0dbd087eb7644d362 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Sat, 13 Apr 2019 13:20:41 -0400 Subject: [PATCH] busses/usb: Handle getting/putting the PCI modules properly. Previously it was possible that we put() them twice, leading to assert-failure panics as their ref-counts would go below 0. Now we get() them once for every bus object that we create, as the destructors put() them. Should fix the panic in #15004. --- src/add-ons/kernel/busses/usb/ehci.cpp | 39 +++++++++++------------- src/add-ons/kernel/busses/usb/ohci.cpp | 42 +++++++++++--------------- src/add-ons/kernel/busses/usb/uhci.cpp | 31 +++++++++++-------- src/add-ons/kernel/busses/usb/xhci.cpp | 32 ++++++++++++-------- 4 files changed, 74 insertions(+), 70 deletions(-) diff --git a/src/add-ons/kernel/busses/usb/ehci.cpp b/src/add-ons/kernel/busses/usb/ehci.cpp index 2e0b6584d5..d857e02a90 100644 --- a/src/add-ons/kernel/busses/usb/ehci.cpp +++ b/src/add-ons/kernel/busses/usb/ehci.cpp @@ -146,17 +146,20 @@ EHCI::AddTo(Stack *stack) TRACE_MODULE("found device at PCI:%d:%d:%d\n", item->bus, item->device, item->function); EHCI *bus = new(std::nothrow) EHCI(item, stack); - if (!bus) { + if (bus == NULL) { delete item; - sPCIModule = NULL; put_module(B_PCI_MODULE_NAME); - if (sPCIx86Module != NULL) { - sPCIx86Module = NULL; + if (sPCIx86Module != NULL) put_module(B_PCI_X86_MODULE_NAME); - } return B_NO_MEMORY; } + // The bus will put the PCI modules when it is destroyed, so get + // them again to increase their reference count. + get_module(B_PCI_MODULE_NAME, (module_info **)&sPCIModule); + if (sPCIx86Module != NULL) + get_module(B_PCI_X86_MODULE_NAME, (module_info **)&sPCIx86Module); + if (bus->InitCheck() != B_OK) { TRACE_MODULE_ERROR("bus failed init check\n"); delete bus; @@ -174,20 +177,16 @@ EHCI::AddTo(Stack *stack) } } - if (!found) { - TRACE_MODULE_ERROR("no devices found\n"); - delete item; - sPCIModule = NULL; - put_module(B_PCI_MODULE_NAME); - if (sPCIx86Module != NULL) { - sPCIx86Module = NULL; - put_module(B_PCI_X86_MODULE_NAME); - } - return ENODEV; - } + // The modules will have been gotten again if we successfully + // initialized a bus, so we should put them here. + put_module(B_PCI_MODULE_NAME); + if (sPCIx86Module != NULL) + put_module(B_PCI_X86_MODULE_NAME); + if (!found) + TRACE_MODULE_ERROR("no devices found\n"); delete item; - return B_OK; + return found ? B_OK : ENODEV; } @@ -690,12 +689,10 @@ EHCI::~EHCI() sPCIx86Module->unconfigure_msi(fPCIInfo->bus, fPCIInfo->device, fPCIInfo->function); } - put_module(B_PCI_MODULE_NAME); - if (sPCIx86Module != NULL) { - sPCIx86Module = NULL; + put_module(B_PCI_MODULE_NAME); + if (sPCIx86Module != NULL) put_module(B_PCI_X86_MODULE_NAME); - } } diff --git a/src/add-ons/kernel/busses/usb/ohci.cpp b/src/add-ons/kernel/busses/usb/ohci.cpp index fc0652f465..6b706b1b40 100644 --- a/src/add-ons/kernel/busses/usb/ohci.cpp +++ b/src/add-ons/kernel/busses/usb/ohci.cpp @@ -97,19 +97,20 @@ OHCI::AddTo(Stack *stack) TRACE_MODULE("found device at PCI:%d:%d:%d\n", item->bus, item->device, item->function); OHCI *bus = new(std::nothrow) OHCI(item, stack); - if (!bus) { + if (bus == NULL) { delete item; - sPCIModule = NULL; put_module(B_PCI_MODULE_NAME); - - if (sPCIx86Module != NULL) { - sPCIx86Module = NULL; + if (sPCIx86Module != NULL) put_module(B_PCI_X86_MODULE_NAME); - } - return B_NO_MEMORY; } + // The bus will put the PCI modules when it is destroyed, so get + // them again to increase their reference count. + get_module(B_PCI_MODULE_NAME, (module_info **)&sPCIModule); + if (sPCIx86Module != NULL) + get_module(B_PCI_X86_MODULE_NAME, (module_info **)&sPCIx86Module); + if (bus->InitCheck() < B_OK) { TRACE_MODULE_ERROR("bus failed init check\n"); delete bus; @@ -127,22 +128,16 @@ OHCI::AddTo(Stack *stack) } } - if (!found) { + // The modules will have been gotten again if we successfully + // initialized a bus, so we should put them here. + put_module(B_PCI_MODULE_NAME); + if (sPCIx86Module != NULL) + put_module(B_PCI_X86_MODULE_NAME); + + if (!found) TRACE_MODULE_ERROR("no devices found\n"); - delete item; - sPCIModule = NULL; - put_module(B_PCI_MODULE_NAME); - - if (sPCIx86Module != NULL) { - sPCIx86Module = NULL; - put_module(B_PCI_X86_MODULE_NAME); - } - - return ENODEV; - } - delete item; - return B_OK; + return found ? B_OK : ENODEV; } @@ -491,11 +486,10 @@ OHCI::~OHCI() sPCIx86Module->unconfigure_msi(fPCIInfo->bus, fPCIInfo->device, fPCIInfo->function); } + put_module(B_PCI_MODULE_NAME); - if (sPCIx86Module != NULL) { - sPCIx86Module = NULL; + if (sPCIx86Module != NULL) put_module(B_PCI_X86_MODULE_NAME); - } } diff --git a/src/add-ons/kernel/busses/usb/uhci.cpp b/src/add-ons/kernel/busses/usb/uhci.cpp index c894cd8785..bbb59f6d89 100644 --- a/src/add-ons/kernel/busses/usb/uhci.cpp +++ b/src/add-ons/kernel/busses/usb/uhci.cpp @@ -336,11 +336,18 @@ UHCI::AddTo(Stack *stack) UHCI *bus = new(std::nothrow) UHCI(item, stack); if (!bus) { delete item; - sPCIModule = NULL; put_module(B_PCI_MODULE_NAME); + if (sPCIx86Module != NULL) + put_module(B_PCI_X86_MODULE_NAME); return B_NO_MEMORY; } + // The bus will put the PCI modules when it is destroyed, so get + // them again to increase their reference count. + get_module(B_PCI_MODULE_NAME, (module_info **)&sPCIModule); + if (sPCIx86Module != NULL) + get_module(B_PCI_X86_MODULE_NAME, (module_info **)&sPCIx86Module); + if (bus->InitCheck() < B_OK) { TRACE_MODULE_ERROR("AddTo(): InitCheck() failed 0x%08" B_PRIx32 "\n", bus->InitCheck()); @@ -359,16 +366,16 @@ UHCI::AddTo(Stack *stack) } } - if (!found) { - TRACE_MODULE_ERROR("no devices found\n"); - delete item; - sPCIModule = NULL; - put_module(B_PCI_MODULE_NAME); - return ENODEV; - } + // The modules will have been gotten again if we successfully + // initialized a bus, so we should put them here. + put_module(B_PCI_MODULE_NAME); + if (sPCIx86Module != NULL) + put_module(B_PCI_X86_MODULE_NAME); + if (!found) + TRACE_MODULE_ERROR("no devices found\n"); delete item; - return B_OK; + return found ? B_OK : ENODEV; } @@ -639,11 +646,11 @@ UHCI::~UHCI() sPCIx86Module->unconfigure_msi(fPCIInfo->bus, fPCIInfo->device, fPCIInfo->function); } + put_module(B_PCI_MODULE_NAME); - if (sPCIx86Module != NULL) { - sPCIx86Module = NULL; + if (sPCIx86Module != NULL) put_module(B_PCI_X86_MODULE_NAME); - } + Unlock(); } diff --git a/src/add-ons/kernel/busses/usb/xhci.cpp b/src/add-ons/kernel/busses/usb/xhci.cpp index 9289f1451b..3cff17ecc3 100644 --- a/src/add-ons/kernel/busses/usb/xhci.cpp +++ b/src/add-ons/kernel/busses/usb/xhci.cpp @@ -121,7 +121,7 @@ XHCI::AddTo(Stack *stack) TRACE_MODULE("searching devices\n"); bool found = false; pci_info *item = new(std::nothrow) pci_info; - if (!item) { + if (item == NULL) { sPCIModule = NULL; put_module(B_PCI_MODULE_NAME); return B_NO_MEMORY; @@ -141,13 +141,21 @@ XHCI::AddTo(Stack *stack) TRACE_MODULE("found device at PCI:%d:%d:%d\n", item->bus, item->device, item->function); XHCI *bus = new(std::nothrow) XHCI(item, stack); - if (!bus) { + if (bus == NULL) { delete item; sPCIModule = NULL; put_module(B_PCI_MODULE_NAME); + if (sPCIx86Module != NULL) + put_module(B_PCI_X86_MODULE_NAME); return B_NO_MEMORY; } + // The bus will put the PCI modules when it is destroyed, so get + // them again to increase their reference count. + get_module(B_PCI_MODULE_NAME, (module_info **)&sPCIModule); + if (sPCIx86Module != NULL) + get_module(B_PCI_X86_MODULE_NAME, (module_info **)&sPCIx86Module); + if (bus->InitCheck() < B_OK) { TRACE_MODULE_ERROR("bus failed init check\n"); delete bus; @@ -165,16 +173,16 @@ XHCI::AddTo(Stack *stack) } } - if (!found) { - TRACE_MODULE_ERROR("no devices found\n"); - delete item; - sPCIModule = NULL; - put_module(B_PCI_MODULE_NAME); - return ENODEV; - } + // The modules will have been gotten again if we successfully + // initialized a bus, so we should put them here. + put_module(B_PCI_MODULE_NAME); + if (sPCIx86Module != NULL) + put_module(B_PCI_X86_MODULE_NAME); + if (!found) + TRACE_MODULE_ERROR("no devices found\n"); delete item; - return B_OK; + return found ? B_OK : ENODEV; } @@ -422,10 +430,8 @@ XHCI::~XHCI() fPCIInfo->device, fPCIInfo->function); } put_module(B_PCI_MODULE_NAME); - if (sPCIx86Module != NULL) { - sPCIx86Module = NULL; + if (sPCIx86Module != NULL) put_module(B_PCI_X86_MODULE_NAME); - } }