Re: [edk2-devel] [PATCH 1/2] BoardModulePkg: Copy device path before processing

["Attar, AbdulLateef (Abdul Lateef)" <AbdulLateef.Attar@amd.com>]

[AMD Official Use Only - General]

Hi Isaac,

Please see inline for my reply. [Abdul]

Thanks
AbduL


-----Original Message-----
From: Oram, Isaac W <isaac.w.oram@intel.com>
Sent: 18 January 2023 02:01
To: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Cc: Dong, Eric <eric.dong@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: RE: [edk2-devel] [PATCH 1/2] BoardModulePkg: Copy device path before processing

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
Line 168: Don't we need to free buffer on this path?
[Abdul] we are freeing all allocated buffers here before returning.

Lines 655, 1083:  Please put a newline between while and the block to free resources.
[Abdul] will make the changes in V3 patch.

Regards,
Isaac

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abdul Lateef Attar via groups.io
Sent: Tuesday, November 15, 2022 4:04 AM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: [edk2-devel] [PATCH 1/2] BoardModulePkg: Copy device path before processing

From: Abdul Lateef Attar <abdattar@amd.com>

GCC compiler puts the DevicePath PCDs to the read-only section. During boot if try to process the device path after PtrGetPtr it throws a page fault exception.

Hence making a local copy using DuplicateDevicePath() to avoid the page fault exception.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
---
 .../Library/BoardBdsHookLib/BoardBdsHookLib.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
index 0bcee7c9a4ba..8700118d255a 100644
--- a/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHookLib.c
+++ b/Platform/Intel/BoardModulePkg/Library/BoardBdsHookLib/BoardBdsHook
+++ Lib.c
@@ -3,6 +3,7 @@
   implementation instance of the BDS hook library



   Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>

+  Copyright (C) 2022 Advanced Micro Devices, Inc. All rights
+ reserved.<BR>

   SPDX-License-Identifier: BSD-2-Clause-Patent



 **/

@@ -131,7 +132,7 @@ IsTrustedConsole (


   switch (ConsoleType) {

     case ConIn:

-      TrustedConsoleDevicepath = PcdGetPtr (PcdTrustedConsoleInputDevicePath);

+      TrustedConsoleDevicepath = DuplicateDevicePath (PcdGetPtr
+ (PcdTrustedConsoleInputDevicePath));

       break;

     case ConOut:

       //

@@ -147,7 +148,7 @@ IsTrustedConsole (
         TempDevicePath = NextDevicePathNode (TempDevicePath);

       }



-      TrustedConsoleDevicepath = PcdGetPtr (PcdTrustedConsoleOutputDevicePath);

+      TrustedConsoleDevicepath = DuplicateDevicePath (PcdGetPtr
+ (PcdTrustedConsoleOutputDevicePath));

       break;

     default:

       ASSERT (FALSE);

@@ -171,7 +172,9 @@ IsTrustedConsole (
   } while (TempDevicePath != NULL);



   FreePool (ConsoleDevice);

-

+  if (TrustedConsoleDevicepath != NULL) {

+    FreePool (TrustedConsoleDevicepath);

+  }

   return FALSE;

 }



@@ -624,7 +627,7 @@ ConnectTrustedStorage (
   EFI_STATUS                Status;

   EFI_HANDLE                DeviceHandle;



-  TrustedStorageDevicepath = PcdGetPtr (PcdTrustedStorageDevicePath);

+  TrustedStorageDevicepath = DuplicateDevicePath (PcdGetPtr
+ (PcdTrustedStorageDevicePath));

   DumpDevicePath (L"TrustedStorage", TrustedStorageDevicepath);



   TempDevicePath = TrustedStorageDevicepath;

@@ -649,6 +652,9 @@ ConnectTrustedStorage (


     FreePool (Instance);

   } while (TempDevicePath != NULL);

+  if (TrustedStorageDevicepath != NULL) {

+    FreePool (TrustedStorageDevicepath);

+  }

 }





@@ -1031,7 +1037,7 @@ AddConsoleVariable (
   EFI_HANDLE                GraphicsControllerHandle;

   EFI_DEVICE_PATH           *GopDevicePath;



-  TempDevicePath = ConsoleDevicePath;

+  TempDevicePath = DuplicateDevicePath (ConsoleDevicePath);

   do {

     Instance = GetNextDevicePathInstance (&TempDevicePath, &Size);

     if (Instance == NULL) {

@@ -1074,6 +1080,9 @@ AddConsoleVariable (


     FreePool (Instance);

   } while (TempDevicePath != NULL);

+  if (TempDevicePath != NULL) {

+    FreePool (TempDevicePath);

+  }

 }





--
2.25.1