Skip to content

Commit 54c3bbe

Browse files
committed
rockchiop/ochi: fix DriverBindingStart error path
`Pages` and `Buf` are freed without being initialized. `OhciFreeDev` internally calls `FreePool`. After the fall through, `Ohc` is used and accessed and then freed again. - replace separate cleanup labels with a single error path - track controller open/install/init state - unmap memory before freeing it - set return status for non-efi erros Bug: #283 Signed-off-by: Pepper Gray <hello@peppergray.xyz>
1 parent de92d3a commit 54c3bbe

1 file changed

Lines changed: 51 additions & 33 deletions

File tree

  • edk2-rockchip/Silicon/Rockchip/Drivers/OhciDxe

‎edk2-rockchip/Silicon/Rockchip/Drivers/OhciDxe/Ohci.c‎

Lines changed: 51 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2052,7 +2052,11 @@ OhciFreeDev (
20522052
UsbHcFreeMemPool (Ohc->MemPool);
20532053
}
20542054

2055-
if (Ohc->HccaMemoryMapping != NULL ) {
2055+
if (Ohc->HccaMemoryMapping != NULL) {
2056+
DmaUnmap (Ohc->HccaMemoryMapping);
2057+
}
2058+
2059+
if (Ohc->HccaMemoryBuf != NULL) {
20562060
DmaFreeBuffer (Ohc->HccaMemoryPages, Ohc->HccaMemoryBuf);
20572061
}
20582062

@@ -2222,6 +2226,9 @@ OHCIDriverBindingStart (
22222226
VOID *Map;
22232227
UINTN Pages;
22242228
UINTN Bytes;
2229+
BOOLEAN DeviceProtocolOpened = FALSE;
2230+
BOOLEAN UsbHcInstalled = FALSE;
2231+
BOOLEAN UsbHcInitialized = FALSE;
22252232

22262233
Ohc = AllocateZeroPool (sizeof (USB_OHCI_HC_DEV));
22272234
if (Ohc == NULL) {
@@ -2243,8 +2250,9 @@ OHCIDriverBindingStart (
22432250
__FUNCTION__,
22442251
Status
22452252
));
2246-
goto FREE_OHC;
2253+
goto ERROR;
22472254
}
2255+
DeviceProtocolOpened = TRUE;
22482256

22492257
Ohc->Signature = USB_OHCI_HC_DEV_SIGNATURE;
22502258

@@ -2275,7 +2283,8 @@ OHCIDriverBindingStart (
22752283

22762284
Ohc->MemPool = UsbHcInitMemPool (TRUE, 0);
22772285
if (Ohc->MemPool == NULL) {
2278-
goto FREE_DEV_BUFFER;
2286+
Status = EFI_OUT_OF_RESOURCES;
2287+
goto ERROR;
22792288
}
22802289

22812290
Bytes = 4096;
@@ -2288,9 +2297,12 @@ OHCIDriverBindingStart (
22882297
);
22892298

22902299
if (EFI_ERROR (Status)) {
2291-
goto FREE_MEM_POOL;
2300+
goto ERROR;
22922301
}
22932302

2303+
Ohc->HccaMemoryBuf = (VOID *)(UINTN)Buf;
2304+
Ohc->HccaMemoryPages = Pages;
2305+
22942306
Status = DmaMap (
22952307
MapOperationBusMasterCommonBuffer,
22962308
Buf,
@@ -2299,14 +2311,17 @@ OHCIDriverBindingStart (
22992311
&Map
23002312
);
23012313

2302-
if (EFI_ERROR (Status) || (Bytes != 4096)) {
2303-
goto FREE_MEM_PAGE;
2314+
if (EFI_ERROR (Status)) {
2315+
goto ERROR;
23042316
}
23052317

23062318
Ohc->HccaMemoryBlock = (HCCA_MEMORY_BLOCK *)(UINTN)PhyAddr;
23072319
Ohc->HccaMemoryMapping = Map;
2308-
Ohc->HccaMemoryBuf = (VOID *)(UINTN)Buf;
2309-
Ohc->HccaMemoryPages = Pages;
2320+
2321+
if (Bytes != 4096) {
2322+
Status = EFI_DEVICE_ERROR;
2323+
goto ERROR;
2324+
}
23102325

23112326
//
23122327
// Install Host Controller Protocol
@@ -2319,8 +2334,9 @@ OHCIDriverBindingStart (
23192334
);
23202335
if (EFI_ERROR (Status)) {
23212336
DEBUG ((DEBUG_INFO, "Install protocol error"));
2322-
goto FREE_OHC;
2337+
goto ERROR;
23232338
}
2339+
UsbHcInstalled = TRUE;
23242340

23252341
//
23262342
// Create event to stop the HC on exit boot services.
@@ -2335,7 +2351,7 @@ OHCIDriverBindingStart (
23352351
);
23362352
if (EFI_ERROR (Status)) {
23372353
DEBUG ((DEBUG_INFO, "Create exit boot event error"));
2338-
goto UNINSTALL_USBHC;
2354+
goto ERROR;
23392355
}
23402356

23412357
//
@@ -2349,19 +2365,20 @@ OHCIDriverBindingStart (
23492365
&Ohc->HouseKeeperTimer
23502366
);
23512367
if (EFI_ERROR (Status)) {
2352-
goto FREE_OHC;
2368+
goto ERROR;
23532369
}
23542370

23552371
Status = OhcInitHC (Ohc);
23562372

23572373
if (EFI_ERROR (Status)) {
23582374
DEBUG ((DEBUG_ERROR, "failed to init ohci host controller\n"));
2359-
goto UNINSTALL_USBHC;
2375+
goto ERROR;
23602376
}
2377+
UsbHcInitialized = TRUE;
23612378

23622379
Status = gBS->SetTimer (Ohc->HouseKeeperTimer, TimerPeriodic, 10 * 1000 * 10);
23632380
if (EFI_ERROR (Status)) {
2364-
goto FREE_OHC;
2381+
goto ERROR;
23652382
}
23662383

23672384
DEBUG ((
@@ -2372,27 +2389,28 @@ OHCIDriverBindingStart (
23722389
));
23732390
return EFI_SUCCESS;
23742391

2375-
FREE_OHC:
2392+
ERROR:
2393+
if (UsbHcInitialized) {
2394+
OhciSetHcControl (Ohc, PERIODIC_ENABLE | CONTROL_ENABLE | ISOCHRONOUS_ENABLE | BULK_ENABLE, 0);
2395+
Ohc->Usb2Hc.SetState (&Ohc->Usb2Hc, EfiUsbHcStateHalt);
2396+
}
2397+
if (UsbHcInstalled) {
2398+
gBS->UninstallMultipleProtocolInterfaces (
2399+
ControllerHandle,
2400+
&gEfiUsb2HcProtocolGuid,
2401+
&Ohc->Usb2Hc,
2402+
NULL
2403+
);
2404+
}
2405+
if (DeviceProtocolOpened) {
2406+
gBS->CloseProtocol (
2407+
ControllerHandle,
2408+
&gOhciDeviceProtocolGuid,
2409+
This->DriverBindingHandle,
2410+
ControllerHandle
2411+
);
2412+
}
23762413
OhciFreeDev (Ohc);
2377-
UNINSTALL_USBHC:
2378-
gBS->UninstallMultipleProtocolInterfaces (
2379-
ControllerHandle,
2380-
&gEfiUsb2HcProtocolGuid,
2381-
&Ohc->Usb2Hc,
2382-
NULL
2383-
);
2384-
gBS->CloseProtocol (
2385-
ControllerHandle,
2386-
&gOhciDeviceProtocolGuid,
2387-
This->DriverBindingHandle,
2388-
ControllerHandle
2389-
);
2390-
FREE_MEM_PAGE:
2391-
DmaFreeBuffer (Pages, Buf);
2392-
FREE_MEM_POOL:
2393-
UsbHcFreeMemPool (Ohc->MemPool);
2394-
FREE_DEV_BUFFER:
2395-
FreePool (Ohc);
23962414

23972415
return Status;
23982416
}

0 commit comments

Comments
 (0)