From 0726d69fa17396cfdf77db262dcfcac27aa8c848 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Tue, 27 Jun 2017 01:34:25 +0200
Subject: [PATCH 1/3] IntelFrameworkModulePkg/IsaBusDxe: fix OpenProtocol()
 usage

The IsaBusControllerDriverStart() function exploits the previous,
non-standard behavior of edk2 that OpenProtocol() outputs the valid
protocol Interface even when OpenProtocol() returns EFI_ALREADY_STARTED.

This behavior has been deleted with commit 45cfcd8dccf8
("MdeModulePkg/DxeCore: Fixed Interface returned by CoreOpenProtocol",
2017-06-23); Interface is no longer set on error (which conforms to the
UEFI spec). As a result, when the OpenProtocol() calls in
IsaBusControllerDriverStart() fail with EFI_ALREADY_STARTED,
ParentDevicePath and IsaAcpi have uninitialized (indeterminate) values,
leading to crashes.

Restore the previous behavior of IsaBusControllerDriverStart() by
explicitly getting the protocol interfaces with
EFI_OPEN_PROTOCOL_GET_PROTOCOL.

Cc: "Gabriel L. Somlo" <gsomlo@gmail.com>
Cc: Amit Kumar <amit.ak@samsung.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Reported-by: "Gabriel L. Somlo" <gsomlo@gmail.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBus.c | 51 +++++++++++++++++-----
 1 file changed, 40 insertions(+), 11 deletions(-)

diff --git a/IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBus.c b/IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBus.c
index 1312f260f9ae..7d996cf86eb1 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBus.c
+++ b/IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBus.c
@@ -252,62 +252,91 @@ IsaBusControllerDriverStart (
                   Controller,
                   EFI_OPEN_PROTOCOL_GET_PROTOCOL
                   );
   if (EFI_ERROR (Status)) {
     return Status;
   }
 
   //
   // Open Device Path Protocol
   //
   Status = gBS->OpenProtocol (
                   Controller,
                   &gEfiDevicePathProtocolGuid,
                   (VOID **) &ParentDevicePath,
                   This->DriverBindingHandle,
                   Controller,
                   EFI_OPEN_PROTOCOL_BY_DRIVER
                   );
-  if (EFI_ERROR (Status) && Status != EFI_ALREADY_STARTED) {
-    return Status;
+  if (EFI_ERROR (Status)) {
+    if  (Status != EFI_ALREADY_STARTED) {
+      return Status;
+    }
+    //
+    // Repeat the above OpenProtocol() call with GET_PROTOCOL.
+    //
+    Status = gBS->OpenProtocol (
+                    Controller,
+                    &gEfiDevicePathProtocolGuid,
+                    (VOID **) &ParentDevicePath,
+                    This->DriverBindingHandle,
+                    Controller,
+                    EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                    );
+    ASSERT_EFI_ERROR (Status);
   }
 
   //
   // Open ISA Acpi Protocol
   //
   Status = gBS->OpenProtocol (
                   Controller,
                   &gEfiIsaAcpiProtocolGuid,
                   (VOID **) &IsaAcpi,
                   This->DriverBindingHandle,
                   Controller,
                   EFI_OPEN_PROTOCOL_BY_DRIVER
                   );
-  if (EFI_ERROR (Status) && Status != EFI_ALREADY_STARTED) {
+  if (EFI_ERROR (Status)) {
+    if (Status != EFI_ALREADY_STARTED) {
+      //
+      // Close opened protocol. (This is required after BY_DRIVER, and not
+      // required but permitted after GET_PROTOCOL.)
+      //
+      gBS->CloseProtocol (
+             Controller,
+             &gEfiDevicePathProtocolGuid,
+             This->DriverBindingHandle,
+             Controller
+             );
+      return Status;
+    }
     //
-    // Close opened protocol
+    // Repeat the above OpenProtocol() call with GET_PROTOCOL.
     //
-    gBS->CloseProtocol (
-           Controller,
-           &gEfiDevicePathProtocolGuid,
-           This->DriverBindingHandle,
-           Controller
-           );
-    return Status;
+    Status = gBS->OpenProtocol (
+                    Controller,
+                    &gEfiIsaAcpiProtocolGuid,
+                    (VOID **) &IsaAcpi,
+                    This->DriverBindingHandle,
+                    Controller,
+                    EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                    );
+    ASSERT_EFI_ERROR (Status);
   }
   //
   // The IsaBus driver will use memory below 16M, which is not tested yet,
   // so call CompatibleRangeTest to test them. Since memory below 1M should
   // be reserved to CSM, and 15M~16M might be reserved for Isa hole, test 1M
   // ~15M here
   //
   Status = gBS->LocateProtocol (
                   &gEfiGenericMemTestProtocolGuid,
                   NULL,
                   (VOID **) &GenMemoryTest
                   );
 
   if (!EFI_ERROR (Status)) {
     Status = GenMemoryTest->CompatibleRangeTest (
                               GenMemoryTest,
                               0x100000,
                               0xE00000
-- 
2.13.1.3.g8be5a757fa67