1. Patchew
  2. EDK2
  3. View series

[edk2] [PATCH] ShellPkg: Fix Shell not able to run startup.nsh from first location

Vladimir Olovyannikov posted 1 patch 7 years, 8 months ago
Failed in applying to current master (apply log)
ShellPkg/Application/Shell/Shell.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
[edk2] [PATCH] ShellPkg: Fix Shell not able to run startup.nsh from first location
Posted by Vladimir Olovyannikov 7 years, 8 months ago 
If startup.nsh is placed into first location (embedded into the firmware image), 
and the current directory has not been set (Internal Shell has just started), 
Shell cannot execute startup because of the bug in the DoStartupScript: 
after finding the correct path of the startup.nsh from the "First location" 
and opening the file, and getting of the file handle, the correct path is 
forgotten, and then RunScriptFile() receives just the name of the file 
(from mStartupScript ariable). 
It then attempts to check if this is a file with ShellIsFile() which fails 
with "EFI_INVALID_PARAMETER" (current directory is NULL, so it cannot get 
fully qualified file name), which causes Shell to exit and unload. 
This patch fixes the issue.
---
 ShellPkg/Application/Shell/Shell.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/ShellPkg/Application/Shell/Shell.c b/ShellPkg/Application/Shell/Shell.c
index 731ba187e4d9..4967fe598448 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -1162,6 +1162,7 @@ DoStartupScript(
   Key.UnicodeChar = CHAR_NULL;
   Key.ScanCode    = 0;
   FileHandle      = NULL;
+  FileStringPath = NULL;
 
   if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.Startup && ShellInfoObject.ShellInitSettings.FileName != NULL) {
     //
@@ -1228,7 +1229,6 @@ DoStartupScript(
   //
   MapName = ShellInfoObject.NewEfiShellProtocol->GetMapFromDevicePath(&ImagePath);
   if (MapName != NULL) {
-    FileStringPath = NULL;
     NewSize = 0;
     FileStringPath = StrnCatGrow(&FileStringPath, &NewSize, MapName, 0);
     if (FileStringPath == NULL) {
@@ -1242,10 +1242,10 @@ DoStartupScript(
       PathRemoveLastItem(FileStringPath);
       FileStringPath = StrnCatGrow(&FileStringPath, &NewSize, mStartupScript, 0);
       Status = ShellInfoObject.NewEfiShellProtocol->OpenFileByName(FileStringPath, &FileHandle, EFI_FILE_MODE_READ);
-      FreePool(FileStringPath);
     }
   }
   if (EFI_ERROR(Status)) {
+    SHELL_FREE_NON_NULL (FileStringPath);
     NamePath = FileDevicePath (NULL, mStartupScript);
     NewPath = AppendDevicePathNode (ImagePath, NamePath);
     FreePool(NamePath);
@@ -1254,15 +1254,21 @@ DoStartupScript(
     // Try the location
     //
     Status = InternalOpenFileDevicePath(NewPath, &FileHandle, EFI_FILE_MODE_READ, 0);
-    FreePool(NewPath);
+    if (!EFI_ERROR (Status)) {
+      FileStringPath = gEfiShellProtocol->GetFilePathFromDevicePath(NewPath);
+      if (FileStringPath == NULL) {
+        Status = EFI_OUT_OF_RESOURCES;
+      }
+    }
+    FreePool (NewPath);
   }
   //
   // If we got a file, run it
   //
   if (!EFI_ERROR(Status) && FileHandle != NULL) {
-    Status = RunScriptFile (mStartupScript, FileHandle, L"", ShellInfoObject.NewShellParametersProtocol);
-    ShellInfoObject.NewEfiShellProtocol->CloseFile(FileHandle);
+    Status = RunScriptFile (FileStringPath, FileHandle, L"", ShellInfoObject.NewShellParametersProtocol);
   } else {
+    SHELL_FREE_NON_NULL (FileStringPath);
     FileStringPath = ShellFindFilePath(mStartupScript);
     if (FileStringPath == NULL) {
       //
@@ -1272,10 +1278,13 @@ DoStartupScript(
       ASSERT(FileHandle == NULL);
     } else {
       Status = RunScriptFile(FileStringPath, NULL, L"", ShellInfoObject.NewShellParametersProtocol);
-      FreePool(FileStringPath);
     }
   }
 
+  SHELL_FREE_NON_NULL (FileStringPath);
+  if (FileHandle != NULL) {
+    ShellInfoObject.NewEfiShellProtocol->CloseFile(FileHandle);
+  }
 
   return (Status);
 }
-- 
1.9.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ShellPkg: Fix Shell not able to run startup.nsh from first location
Posted by Carsey, Jaben 7 years, 8 months ago 
Looks good to me.  

Ray?

> -----Original Message-----
> From: Vladimir Olovyannikov [mailto:vladimir.olovyannikov@broadcom.com]
> Sent: Monday, March 06, 2017 10:15 AM
> To: edk2-devel@lists.01.org; Carsey, Jaben <jaben.carsey@intel.com>; Ni,
> Ruiyu <ruiyu.ni@intel.com>
> Cc: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> Subject: [PATCH] ShellPkg: Fix Shell not able to run startup.nsh from first
> location
> Importance: High
> 
> If startup.nsh is placed into first location (embedded into the firmware
> image),
> and the current directory has not been set (Internal Shell has just started),
> Shell cannot execute startup because of the bug in the DoStartupScript:
> after finding the correct path of the startup.nsh from the "First location"
> and opening the file, and getting of the file handle, the correct path is
> forgotten, and then RunScriptFile() receives just the name of the file
> (from mStartupScript ariable).
> It then attempts to check if this is a file with ShellIsFile() which fails
> with "EFI_INVALID_PARAMETER" (current directory is NULL, so it cannot get
> fully qualified file name), which causes Shell to exit and unload.
> This patch fixes the issue.
> ---
>  ShellPkg/Application/Shell/Shell.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/ShellPkg/Application/Shell/Shell.c
> b/ShellPkg/Application/Shell/Shell.c
> index 731ba187e4d9..4967fe598448 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -1162,6 +1162,7 @@ DoStartupScript(
>    Key.UnicodeChar = CHAR_NULL;
>    Key.ScanCode    = 0;
>    FileHandle      = NULL;
> +  FileStringPath = NULL;
> 
>    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.Startup &&
> ShellInfoObject.ShellInitSettings.FileName != NULL) {
>      //
> @@ -1228,7 +1229,6 @@ DoStartupScript(
>    //
>    MapName = ShellInfoObject.NewEfiShellProtocol-
> >GetMapFromDevicePath(&ImagePath);
>    if (MapName != NULL) {
> -    FileStringPath = NULL;
>      NewSize = 0;
>      FileStringPath = StrnCatGrow(&FileStringPath, &NewSize, MapName, 0);
>      if (FileStringPath == NULL) {
> @@ -1242,10 +1242,10 @@ DoStartupScript(
>        PathRemoveLastItem(FileStringPath);
>        FileStringPath = StrnCatGrow(&FileStringPath, &NewSize, mStartupScript,
> 0);
>        Status = ShellInfoObject.NewEfiShellProtocol-
> >OpenFileByName(FileStringPath, &FileHandle, EFI_FILE_MODE_READ);
> -      FreePool(FileStringPath);
>      }
>    }
>    if (EFI_ERROR(Status)) {
> +    SHELL_FREE_NON_NULL (FileStringPath);
>      NamePath = FileDevicePath (NULL, mStartupScript);
>      NewPath = AppendDevicePathNode (ImagePath, NamePath);
>      FreePool(NamePath);
> @@ -1254,15 +1254,21 @@ DoStartupScript(
>      // Try the location
>      //
>      Status = InternalOpenFileDevicePath(NewPath, &FileHandle,
> EFI_FILE_MODE_READ, 0);
> -    FreePool(NewPath);
> +    if (!EFI_ERROR (Status)) {
> +      FileStringPath = gEfiShellProtocol-
> >GetFilePathFromDevicePath(NewPath);
> +      if (FileStringPath == NULL) {
> +        Status = EFI_OUT_OF_RESOURCES;
> +      }
> +    }
> +    FreePool (NewPath);
>    }
>    //
>    // If we got a file, run it
>    //
>    if (!EFI_ERROR(Status) && FileHandle != NULL) {
> -    Status = RunScriptFile (mStartupScript, FileHandle, L"",
> ShellInfoObject.NewShellParametersProtocol);
> -    ShellInfoObject.NewEfiShellProtocol->CloseFile(FileHandle);
> +    Status = RunScriptFile (FileStringPath, FileHandle, L"",
> ShellInfoObject.NewShellParametersProtocol);
>    } else {
> +    SHELL_FREE_NON_NULL (FileStringPath);
>      FileStringPath = ShellFindFilePath(mStartupScript);
>      if (FileStringPath == NULL) {
>        //
> @@ -1272,10 +1278,13 @@ DoStartupScript(
>        ASSERT(FileHandle == NULL);
>      } else {
>        Status = RunScriptFile(FileStringPath, NULL, L"",
> ShellInfoObject.NewShellParametersProtocol);
> -      FreePool(FileStringPath);
>      }
>    }
> 
> +  SHELL_FREE_NON_NULL (FileStringPath);
> +  if (FileHandle != NULL) {
> +    ShellInfoObject.NewEfiShellProtocol->CloseFile(FileHandle);
> +  }
> 
>    return (Status);
>  }
> --
> 1.9.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.