In QemuVideoControllerDriverStart():
- remove redundant zero-initialization of:
- Private->Handle (2 locations)
- Private->GopDevicePath (when at devpath end)
- remove fields used for error handling only:
- PciAttributesSaved
- tigthen scope of temporaries:
- MmioDesc
- AcpiDeviceNode
- supplement missing error checks:
- AppendDevicePathNode() can fail with out-of-memory (2 locations)
- when installing GopDevicePath
- retval of QemuVideoGraphicsOutputConstructor() (can justifiedly fail
with out-of-resources)
- plug leaks on error:
- free GopDevicePath (AppendDevicePathNode() allocates dynamically)
- uninstall GopDevicePath
- free Private->ModeData
- call QemuVideoGraphicsOutputDestructor()
- uninstall GOP
In QemuVideoGraphicsOutputConstructor(), called by Start():
- supplement missing error checks:
- QemuVideoGraphicsOutputSetMode() retval (it can fail with
out-of-resources)
- plug leaks on error:
- free Mode->Info
- free Mode
In QemuVideoCirrusModeSetup() and QemuVideoBochsModeSetup(), both called
by Start():
- supplement missing error checks:
- AllocatePool() can fail in both
In QemuVideoGraphicsOutputDestructor(), called by Start() on the error
path:
- plug leaks:
- free Private->LineBuffer, which is allocated in
Start() -> Constructor() -> SetMode()
In QemuVideoGraphicsOutputSetMode(), called by Start() indirectly:
- remove redundant zero-assignment to:
- Private->LineBuffer
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
---
OvmfPkg/QemuVideoDxe/Driver.c | 207 +++++++++++++++++++++-----------------
OvmfPkg/QemuVideoDxe/Gop.c | 23 ++++-
OvmfPkg/QemuVideoDxe/Initialize.c | 6 ++
3 files changed, 140 insertions(+), 96 deletions(-)
diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
index b253ec7..b1105f4 100644
--- a/OvmfPkg/QemuVideoDxe/Driver.c
+++ b/OvmfPkg/QemuVideoDxe/Driver.c
@@ -203,29 +203,23 @@ QemuVideoControllerDriverStart (
{
EFI_STATUS Status;
QEMU_VIDEO_PRIVATE_DATA *Private;
- BOOLEAN PciAttributesSaved;
EFI_DEVICE_PATH_PROTOCOL *ParentDevicePath;
- ACPI_ADR_DEVICE_PATH AcpiDeviceNode;
PCI_TYPE00 Pci;
QEMU_VIDEO_CARD *Card;
- EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *MmioDesc;
EFI_PCI_IO_PROTOCOL *ChildPciIo;
- PciAttributesSaved = FALSE;
//
// Allocate Private context data for GOP inteface.
//
Private = AllocateZeroPool (sizeof (QEMU_VIDEO_PRIVATE_DATA));
if (Private == NULL) {
- Status = EFI_OUT_OF_RESOURCES;
- goto Error;
+ return EFI_OUT_OF_RESOURCES;
}
//
// Set up context record
//
Private->Signature = QEMU_VIDEO_PRIVATE_DATA_SIGNATURE;
- Private->Handle = NULL;
//
// Open PCI I/O Protocol
@@ -239,7 +233,7 @@ QemuVideoControllerDriverStart (
EFI_OPEN_PROTOCOL_BY_DRIVER
);
if (EFI_ERROR (Status)) {
- goto Error;
+ goto FreePrivate;
}
//
@@ -253,13 +247,16 @@ QemuVideoControllerDriverStart (
&Pci
);
if (EFI_ERROR (Status)) {
- goto Error;
+ goto ClosePciIo;
}
+ //
+ // Determine card variant.
+ //
Card = QemuVideoDetect(Pci.Hdr.VendorId, Pci.Hdr.DeviceId);
if (Card == NULL) {
Status = EFI_DEVICE_ERROR;
- goto Error;
+ goto ClosePciIo;
}
Private->Variant = Card->Variant;
@@ -274,10 +271,12 @@ QemuVideoControllerDriverStart (
);
if (EFI_ERROR (Status)) {
- goto Error;
+ goto ClosePciIo;
}
- PciAttributesSaved = TRUE;
+ //
+ // Set new PCI attributes
+ //
Status = Private->PciIo->Attributes (
Private->PciIo,
EfiPciIoAttributeOperationEnable,
@@ -285,13 +284,15 @@ QemuVideoControllerDriverStart (
NULL
);
if (EFI_ERROR (Status)) {
- goto Error;
+ goto ClosePciIo;
}
//
// Check whenever the qemu stdvga mmio bar is present (qemu 1.3+).
//
if (Private->Variant == QEMU_VIDEO_BOCHS_MMIO) {
+ EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *MmioDesc;
+
Status = Private->PciIo->GetBarAttributes (
Private->PciIo,
PCI_BAR_IDX2,
@@ -322,7 +323,7 @@ QemuVideoControllerDriverStart (
if ((BochsId & 0xFFF0) != VBE_DISPI_ID0) {
DEBUG ((EFI_D_INFO, "QemuVideo: BochsID mismatch (got 0x%x)\n",
BochsId));
Status = EFI_DEVICE_ERROR;
- goto Error;
+ goto RestoreAttributes;
}
}
@@ -335,13 +336,15 @@ QemuVideoControllerDriverStart (
(VOID **) &ParentDevicePath
);
if (EFI_ERROR (Status)) {
- goto Error;
+ goto RestoreAttributes;
}
//
// Set Gop Device Path
//
if (RemainingDevicePath == NULL) {
+ ACPI_ADR_DEVICE_PATH AcpiDeviceNode;
+
ZeroMem (&AcpiDeviceNode, sizeof (ACPI_ADR_DEVICE_PATH));
AcpiDeviceNode.Header.Type = ACPI_DEVICE_PATH;
AcpiDeviceNode.Header.SubType = ACPI_ADR_DP;
@@ -352,31 +355,37 @@ QemuVideoControllerDriverStart (
ParentDevicePath,
(EFI_DEVICE_PATH_PROTOCOL *)
&AcpiDeviceNode
);
+ if (Private->GopDevicePath == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto RestoreAttributes;
+ }
} else if (!IsDevicePathEnd (RemainingDevicePath)) {
//
// If RemainingDevicePath isn't the End of Device Path Node,
// only scan the specified device by RemainingDevicePath
//
Private->GopDevicePath = AppendDevicePathNode (ParentDevicePath,
RemainingDevicePath);
- } else {
- //
- // If RemainingDevicePath is the End of Device Path Node,
- // don't create child device and return EFI_SUCCESS
- //
- Private->GopDevicePath = NULL;
+ if (Private->GopDevicePath == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto RestoreAttributes;
+ }
}
-
+
+ //
+ // Create new child handle and install the device path protocol on it only if
+ // RemainingDevicePath equals NULL, or doesn't point to the End of Device
+ // Path Node.
+ //
if (Private->GopDevicePath != NULL) {
- //
- // Creat child handle and device path protocol firstly
- //
- Private->Handle = NULL;
Status = gBS->InstallMultipleProtocolInterfaces (
&Private->Handle,
&gEfiDevicePathProtocolGuid,
Private->GopDevicePath,
NULL
);
+ if (EFI_ERROR (Status)) {
+ goto FreeGopDevicePath;
+ }
}
//
@@ -397,84 +406,96 @@ QemuVideoControllerDriverStart (
break;
}
if (EFI_ERROR (Status)) {
- goto Error;
+ goto UninstallGopDevicePath;
}
+ //
+ // If RemainingDevicePath points to the End of Device Path Node, then we
+ // haven't created a child handle, and we're done.
+ //
if (Private->GopDevicePath == NULL) {
- //
- // If RemainingDevicePath is the End of Device Path Node,
- // don't create child device and return EFI_SUCCESS
- //
- Status = EFI_SUCCESS;
- } else {
+ return EFI_SUCCESS;
+ }
- //
- // Start the GOP software stack.
- //
- Status = QemuVideoGraphicsOutputConstructor (Private);
- ASSERT_EFI_ERROR (Status);
+ //
+ // Start the GOP software stack.
+ //
+ Status = QemuVideoGraphicsOutputConstructor (Private);
+ if (EFI_ERROR (Status)) {
+ goto FreeModeData;
+ }
- Status = gBS->InstallMultipleProtocolInterfaces (
- &Private->Handle,
- &gEfiGraphicsOutputProtocolGuid,
- &Private->GraphicsOutput,
- NULL
- );
- if (EFI_ERROR (Status)) {
- goto Error;
- }
-
- Status = gBS->OpenProtocol (
- Controller,
- &gEfiPciIoProtocolGuid,
- (VOID **) &ChildPciIo,
- This->DriverBindingHandle,
- Private->Handle,
- EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
+ Status = gBS->InstallMultipleProtocolInterfaces (
+ &Private->Handle,
+ &gEfiGraphicsOutputProtocolGuid,
+ &Private->GraphicsOutput,
+ NULL
);
-
- if (EFI_ERROR (Status)) {
- goto Error;
- }
+ if (EFI_ERROR (Status)) {
+ goto DestructQemuVideoGraphics;
}
-Error:
+ //
+ // Reference parent handle from child handle.
+ //
+ Status = gBS->OpenProtocol (
+ Controller,
+ &gEfiPciIoProtocolGuid,
+ (VOID **) &ChildPciIo,
+ This->DriverBindingHandle,
+ Private->Handle,
+ EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
+ );
if (EFI_ERROR (Status)) {
- if (Private) {
- if (Private->PciIo) {
- if (PciAttributesSaved == TRUE) {
- //
- // Restore original PCI attributes
- //
- Private->PciIo->Attributes (
- Private->PciIo,
- EfiPciIoAttributeOperationSet,
- Private->OriginalPciAttributes,
- NULL
- );
- }
- //
- // Close the PCI I/O Protocol
- //
- gBS->CloseProtocol (
- Controller,
- &gEfiPciIoProtocolGuid,
- This->DriverBindingHandle,
- Controller
- );
-
- gBS->CloseProtocol (
- Controller,
- &gEfiPciIoProtocolGuid,
- This->DriverBindingHandle,
- Private->Handle
- );
- }
-
- gBS->FreePool (Private);
- }
+ goto UninstallGop;
}
+ //
+ // When adding further code that can fail, the error handling label to jump
+ // to is CloseChildRef.
+ //
+ return EFI_SUCCESS;
+
+#if 0 // in order to suppress the compiler warning about the unused label
+CloseChildRef:
+#endif
+ gBS->CloseProtocol (Controller, &gEfiPciIoProtocolGuid,
+ This->DriverBindingHandle, Private->Handle);
+
+UninstallGop:
+ gBS->UninstallProtocolInterface (Private->Handle,
+ &gEfiGraphicsOutputProtocolGuid, &Private->GraphicsOutput);
+
+DestructQemuVideoGraphics:
+ QemuVideoGraphicsOutputDestructor (Private);
+
+FreeModeData:
+ FreePool (Private->ModeData);
+
+UninstallGopDevicePath:
+ //
+ // Handles the case transparently when Private->Handle and
+ // Private->GopDevicePath are NULL.
+ //
+ gBS->UninstallProtocolInterface (Private->Handle,
+ &gEfiDevicePathProtocolGuid, Private->GopDevicePath);
+
+FreeGopDevicePath:
+ if (Private->GopDevicePath != NULL) {
+ FreePool (Private->GopDevicePath);
+ }
+
+RestoreAttributes:
+ Private->PciIo->Attributes (Private->PciIo, EfiPciIoAttributeOperationSet,
+ Private->OriginalPciAttributes, NULL);
+
+ClosePciIo:
+ gBS->CloseProtocol (Controller, &gEfiPciIoProtocolGuid,
+ This->DriverBindingHandle, Controller);
+
+FreePrivate:
+ FreePool (Private);
+
return Status;
}
diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
index 30aac7f..1b7db32 100644
--- a/OvmfPkg/QemuVideoDxe/Gop.c
+++ b/OvmfPkg/QemuVideoDxe/Gop.c
@@ -176,7 +176,6 @@ Routine Description:
gBS->FreePool (Private->LineBuffer);
}
- Private->LineBuffer = NULL;
Private->LineBuffer = AllocatePool (4 * ModeData->HorizontalResolution);
if (Private->LineBuffer == NULL) {
return EFI_OUT_OF_RESOURCES;
@@ -321,13 +320,14 @@ QemuVideoGraphicsOutputConstructor (
if (EFI_ERROR (Status)) {
return Status;
}
+
Status = gBS->AllocatePool (
EfiBootServicesData,
sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION),
(VOID **) &Private->GraphicsOutput.Mode->Info
);
if (EFI_ERROR (Status)) {
- return Status;
+ goto FreeMode;
}
Private->GraphicsOutput.Mode->MaxMode = (UINT32) Private->MaxMode;
Private->GraphicsOutput.Mode->Mode = GRAPHICS_OUTPUT_INVALIDE_MODE_NUMBER;
@@ -337,7 +337,11 @@ QemuVideoGraphicsOutputConstructor (
//
// Initialize the hardware
//
- GraphicsOutput->SetMode (GraphicsOutput, 0);
+ Status = GraphicsOutput->SetMode (GraphicsOutput, 0);
+ if (EFI_ERROR (Status)) {
+ goto FreeInfo;
+ }
+
DrawLogo (
Private,
Private->ModeData[Private->GraphicsOutput.Mode->Mode].HorizontalResolution,
@@ -345,6 +349,15 @@ QemuVideoGraphicsOutputConstructor (
);
return EFI_SUCCESS;
+
+FreeInfo:
+ FreePool (Private->GraphicsOutput.Mode->Info);
+
+FreeMode:
+ FreePool (Private->GraphicsOutput.Mode);
+ Private->GraphicsOutput.Mode = NULL;
+
+ return Status;
}
EFI_STATUS
@@ -363,6 +376,10 @@ Returns:
--*/
{
+ if (Private->LineBuffer != NULL) {
+ FreePool (Private->LineBuffer);
+ }
+
if (Private->GraphicsOutput.Mode != NULL) {
if (Private->GraphicsOutput.Mode->Info != NULL) {
gBS->FreePool (Private->GraphicsOutput.Mode->Info);
diff --git a/OvmfPkg/QemuVideoDxe/Initialize.c
b/OvmfPkg/QemuVideoDxe/Initialize.c
index 305797b..3774478 100644
--- a/OvmfPkg/QemuVideoDxe/Initialize.c
+++ b/OvmfPkg/QemuVideoDxe/Initialize.c
@@ -176,6 +176,9 @@ QemuVideoCirrusModeSetup (
Private->ModeData = AllocatePool (
sizeof (Private->ModeData[0]) *
QEMU_VIDEO_CIRRUS_MODE_COUNT
);
+ if (Private->ModeData == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
ModeData = Private->ModeData;
VideoMode = &QemuVideoCirrusModes[0];
for (Index = 0; Index < QEMU_VIDEO_CIRRUS_MODE_COUNT; Index ++) {
@@ -228,6 +231,9 @@ QemuVideoBochsModeSetup (
Private->ModeData = AllocatePool (
sizeof (Private->ModeData[0]) *
QEMU_VIDEO_BOCHS_MODE_COUNT
);
+ if (Private->ModeData == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
ModeData = Private->ModeData;
VideoMode = &QemuVideoBochsModes[0];
for (Index = 0; Index < QEMU_VIDEO_BOCHS_MODE_COUNT; Index ++) {
--
1.8.3.1
------------------------------------------------------------------------------
Managing the Performance of Cloud-Based Applications
Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
Read the Whitepaper.
http://pubads.g.doubleclick.net/gampad/clk?id=121054471&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
