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

The SerialControllerDriverStart() 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
SerialControllerDriverStart() fail with EFI_ALREADY_STARTED,
ParentDevicePath and IsaIo have uninitialized (indeterminate) values,
leading to a failed assertion later.

Restore the previous behavior of SerialControllerDriverStart() 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>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 .../Bus/Isa/IsaSerialDxe/Serial.c                  | 36 ++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c
index 57ee669d1406..48836f6fee92 100644
--- a/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c
+++ b/IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/Serial.c
@@ -409,73 +409,100 @@ SerialControllerDriverStart (
   UART_DEVICE_PATH                    *Uart;
   UINT32                              FlowControlMap;
   UART_FLOW_CONTROL_DEVICE_PATH       *FlowControl;
   EFI_DEVICE_PATH_PROTOCOL            *TempDevicePath;
   UINT32                              Control;
 
   SerialDevice = NULL;
   //
   // Get the Parent Device Path
   //
   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);
   }
   //
   // Report status code enable the serial
   //
   REPORT_STATUS_CODE_WITH_DEVICE_PATH (
     EFI_PROGRESS_CODE,
     EFI_P_PC_ENABLE | EFI_PERIPHERAL_SERIAL_PORT,
     ParentDevicePath
     );
 
   //
   // Grab the IO abstraction we need to get any work done
   //
   Status = gBS->OpenProtocol (
                   Controller,
                   &gEfiIsaIoProtocolGuid,
                   (VOID **) &IsaIo,
                   This->DriverBindingHandle,
                   Controller,
                   EFI_OPEN_PROTOCOL_BY_DRIVER
                   );
   if (EFI_ERROR (Status) && Status != EFI_ALREADY_STARTED) {
     goto Error;
   }
 
   if (Status == EFI_ALREADY_STARTED) {
 
     if (RemainingDevicePath == NULL || IsDevicePathEnd (RemainingDevicePath)) {
       //
       // If RemainingDevicePath is NULL or is the End of Device Path Node
       //
       return EFI_SUCCESS;
     }
 
     //
+    // Repeat the above OpenProtocol() call with GET_PROTOCOL.
+    //
+    Status = gBS->OpenProtocol (
+                    Controller,
+                    &gEfiIsaIoProtocolGuid,
+                    (VOID **) &IsaIo,
+                    This->DriverBindingHandle,
+                    Controller,
+                    EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                    );
+    ASSERT_EFI_ERROR (Status);
+
+    //
     // Make sure a child handle does not already exist.  This driver can only
     // produce one child per serial port.
     //
     Status = gBS->OpenProtocolInformation (
                     Controller,
                     &gEfiIsaIoProtocolGuid,
                     &OpenInfoBuffer,
                     &EntryCount
                     );
     if (EFI_ERROR (Status)) {
       return Status;
     }
 
     Status = EFI_ALREADY_STARTED;
     for (Index = 0; Index < EntryCount; Index++) {
       if ((OpenInfoBuffer[Index].Attributes & EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER) != 0) {
         Status = gBS->OpenProtocol (
                         OpenInfoBuffer[Index].ControllerHandle,
@@ -659,36 +686,41 @@ SerialControllerDriverStart (
                   );
   if (EFI_ERROR (Status)) {
     goto Error;
   }
   //
   // Open For Child Device
   //
   Status = gBS->OpenProtocol (
                   Controller,
                   &gEfiIsaIoProtocolGuid,
                   (VOID **) &IsaIo,
                   This->DriverBindingHandle,
                   SerialDevice->Handle,
                   EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
                   );
 
 Error:
   if (EFI_ERROR (Status)) {
+    //
+    // Closing the protocol interfaces opened with BY_DRIVER is required.
+    // Closing the protocol interfaces opened with GET_PROTOCOL is not required
+    // but permitted.
+    //
     gBS->CloseProtocol (
            Controller,
            &gEfiDevicePathProtocolGuid,
            This->DriverBindingHandle,
            Controller
            );
     gBS->CloseProtocol (
            Controller,
            &gEfiIsaIoProtocolGuid,
            This->DriverBindingHandle,
            Controller
            );
     if (SerialDevice != NULL) {
       if (SerialDevice->DevicePath != NULL) {
         gBS->FreePool (SerialDevice->DevicePath);
       }
 
       FreeUnicodeStringTable (SerialDevice->ControllerNameTable);
-- 
2.13.1.3.g8be5a757fa67