ArmPkg/SemihostFs: Fix open file issues revealed by SCT

Fix return codes in case of errors.
Fix translation from EFI open mode to semi-hosting open mode to avoid the
truncate to zero length of an already existing file opened in read/write
or read/write/create mode.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Reviewed-By: Olivier Martin <olivier.martin@arm.com>



git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16241 6f19259b-4bc3-4df7-8a09-765794883524

ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.c

@@ -57,7 +57,7 @@ EFI_FILE gSemihostFsFile = {
 };
 
 //
-// Device path for SemiHosting. It contains our autogened Caller ID GUID.
+// Device path for semi-hosting. It contains our autogened Caller ID GUID.
 //
 typedef struct {
   VENDOR_DEVICE_PATH        Guid;
@@ -149,100 +149,151 @@ VolumeOpen (
   return EFI_SUCCESS;
 }
 
+/**
+  Open a file on the host system by means of the semihosting interface.
+
+  @param[in]   This        A pointer to the EFI_FILE_PROTOCOL instance that is
+                           the file handle to source location.
+  @param[out]  NewHandle   A pointer to the location to return the opened
+                           handle for the new file.
+  @param[in]   FileName    The Null-terminated string of the name of the file
+                           to be opened.
+  @param[in]   OpenMode    The mode to open the file : Read or Read/Write or
+                           Read/Write/Create
+  @param[in]   Attributes  Only valid for EFI_FILE_MODE_CREATE, in which case these
+                           are the attribute bits for the newly created file. The
+                           mnemonics of the attribute bits are : EFI_FILE_READ_ONLY,
+                           EFI_FILE_HIDDEN, EFI_FILE_SYSTEM, EFI_FILE_RESERVED,
+                           EFI_FILE_DIRECTORY and EFI_FILE_ARCHIVE.
+
+  @retval  EFI_SUCCESS            The file was open.
+  @retval  EFI_NOT_FOUND          The specified file could not be found.
+  @retval  EFI_DEVICE_ERROR       The last issued semi-hosting operation failed.
+  @retval  EFI_WRITE_PROTECTED    Attempt to create a directory. This is not possible
+                                  with the semi-hosting interface.
+  @retval  EFI_OUT_OF_RESOURCES   Not enough resources were available to open the file.
+  @retval  EFI_INVALID_PARAMETER  At least one of the parameters is invalid.
+
+**/
 EFI_STATUS
 FileOpen (
-  IN  EFI_FILE    *File,
+  IN  EFI_FILE  *This,
-  OUT EFI_FILE    **NewHandle,
+  OUT EFI_FILE  **NewHandle,
-  IN  CHAR16      *FileName,
+  IN  CHAR16    *FileName,
-  IN  UINT64      OpenMode,
+  IN  UINT64    OpenMode,
-  IN  UINT64      Attributes
+  IN  UINT64    Attributes
   )
 {
-  SEMIHOST_FCB  *FileFcb = NULL;
+  SEMIHOST_FCB   *FileFcb;
-  EFI_STATUS    Status   = EFI_SUCCESS;
+  RETURN_STATUS  Return;
-  UINTN         SemihostHandle;
+  EFI_STATUS     Status;
-  CHAR8         *AsciiFileName;
+  UINTN          SemihostHandle;
-  UINT32        SemihostMode;
+  CHAR8          *AsciiFileName;
-  BOOLEAN       IsRoot;
+  UINT32         SemihostMode;
-  UINTN         Length;
+  UINTN          Length;
 
   if ((FileName == NULL) || (NewHandle == NULL)) {
     return EFI_INVALID_PARAMETER;
   }
 
-  // Semihosting does not support directories
+  if ( (OpenMode != EFI_FILE_MODE_READ) &&
-  if (Attributes & EFI_FILE_DIRECTORY) {
+       (OpenMode != (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE)) &&
-    return EFI_UNSUPPORTED;
+       (OpenMode != (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE | EFI_FILE_MODE_CREATE)) ) {
+    return EFI_INVALID_PARAMETER;
   }
 
-  // Semihost interface requires ASCII filenames
+  if ((OpenMode & EFI_FILE_MODE_CREATE) &&
-  AsciiFileName = AllocatePool ((StrLen (FileName) + 1) * sizeof (CHAR8));
+      (Attributes & EFI_FILE_DIRECTORY)    ) {
+    return EFI_WRITE_PROTECTED;
+  }
+
+  AsciiFileName = AllocatePool (StrLen (FileName) + 1);
   if (AsciiFileName == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
   UnicodeStrToAsciiStr (FileName, AsciiFileName);
 
+  // Opening '/', '\', '.', or the NULL pathname is trying to open the root directory
   if ((AsciiStrCmp (AsciiFileName, "\\") == 0) ||
       (AsciiStrCmp (AsciiFileName, "/")  == 0) ||
       (AsciiStrCmp (AsciiFileName, "")   == 0) ||
-      (AsciiStrCmp (AsciiFileName, ".")  == 0)) {
+      (AsciiStrCmp (AsciiFileName, ".")  == 0)    ) {
-    // Opening '/', '\', '.', or the NULL pathname is trying to open the root directory
-    IsRoot = TRUE;
-
-    // Root directory node doesn't have a name.
     FreePool (AsciiFileName);
-    AsciiFileName = NULL;
+    return (VolumeOpen (&gSemihostFs, NewHandle));
+  }
+
+  //
+  // No control is done here concerning the file path. It is passed
+  // as it is to the host operating system through the semi-hosting
+  // interface. We first try to open the file in the read or update
+  // mode even if the file creation has been asked for. That way, if
+  // the file already exists, it is not truncated to zero length. In
+  // write mode (bit SEMIHOST_FILE_MODE_WRITE up), if the file already
+  // exists, it is reset to an empty file.
+  //
+  if (OpenMode == EFI_FILE_MODE_READ) {
+    SemihostMode = SEMIHOST_FILE_MODE_READ | SEMIHOST_FILE_MODE_BINARY;
   } else {
-    // Translate EFI_FILE_MODE into Semihosting mode
+    SemihostMode = SEMIHOST_FILE_MODE_READ | SEMIHOST_FILE_MODE_BINARY | SEMIHOST_FILE_MODE_UPDATE;
-    if (OpenMode & EFI_FILE_MODE_WRITE) {
+  }
-      SemihostMode = SEMIHOST_FILE_MODE_WRITE | SEMIHOST_FILE_MODE_BINARY;
+  Return = SemihostFileOpen (AsciiFileName, SemihostMode, &SemihostHandle);
-    } else if (OpenMode & EFI_FILE_MODE_READ) {
-      SemihostMode = SEMIHOST_FILE_MODE_READ  | SEMIHOST_FILE_MODE_BINARY;
-    } else {
-      return EFI_UNSUPPORTED;
-    }
 
-    // Add the creation flag if necessary
+  if (RETURN_ERROR (Return)) {
     if (OpenMode & EFI_FILE_MODE_CREATE) {
-      SemihostMode |= SEMIHOST_FILE_MODE_UPDATE;
+      //
+      // In the create if does not exist case, if the opening in update
+      // mode failed, create it and open it in update mode. The update
+      // mode allows for both read and write from and to the file.
+      //
+      Return = SemihostFileOpen (
+                 AsciiFileName,
+                 SEMIHOST_FILE_MODE_WRITE | SEMIHOST_FILE_MODE_BINARY | SEMIHOST_FILE_MODE_UPDATE,
+                 &SemihostHandle
+                 );
+      if (RETURN_ERROR (Return)) {
+        Status = EFI_DEVICE_ERROR;
+        goto Error;
+      }
+    } else {
+      Status = EFI_NOT_FOUND;
+      goto Error;
     }
-
-    // Call the semihosting interface to open the file.
-    Status = SemihostFileOpen (AsciiFileName, SemihostMode, &SemihostHandle);
-    if (EFI_ERROR(Status)) {
-      return Status;
-    }
-
-    IsRoot = FALSE;
   }
 
   // Allocate a control block and fill it
   FileFcb = AllocateFCB ();
   if (FileFcb == NULL) {
-    return EFI_OUT_OF_RESOURCES;
+    Status = EFI_OUT_OF_RESOURCES;
+    goto Error;
   }
 
   FileFcb->FileName       = AsciiFileName;
   FileFcb->SemihostHandle = SemihostHandle;
   FileFcb->Position       = 0;
-  FileFcb->IsRoot         = IsRoot;
+  FileFcb->IsRoot         = 0;
   FileFcb->OpenMode       = OpenMode;
 
-  if (!IsRoot) {
+  Return = SemihostFileLength (SemihostHandle, &Length);
-    Status = SemihostFileLength (SemihostHandle, &Length);
+  if (RETURN_ERROR (Return)) {
-    if (EFI_ERROR(Status)) {
+    Status = EFI_DEVICE_ERROR;
-      return Status;
+    FreeFCB (FileFcb);
-    }
+    goto Error;
-
-    FileFcb->Info.FileSize     = Length;
-    FileFcb->Info.PhysicalSize = Length;
-    FileFcb->Info.Attribute    = Attributes;
   }
 
+  FileFcb->Info.FileSize     = Length;
+  FileFcb->Info.PhysicalSize = Length;
+  FileFcb->Info.Attribute    = (OpenMode & EFI_FILE_MODE_CREATE) ? Attributes : 0;
+
   InsertTailList (&gFileList, &FileFcb->Link);
 
   *NewHandle = &FileFcb->File;
 
+  return EFI_SUCCESS;
+
+Error:
+
+  FreePool (AsciiFileName);
+
   return Status;
 }
 

ArmPkg/Filesystem/SemihostFs/Arm/SemihostFs.h

@@ -22,9 +22,35 @@ VolumeOpen (
   OUT EFI_FILE                        **Root
   );
 
+/**
+  Open a file on the host system by means of the semihosting interface.
+
+  @param[in]   This        A pointer to the EFI_FILE_PROTOCOL instance that is
+                           the file handle to source location.
+  @param[out]  NewHandle   A pointer to the location to return the opened
+                           handle for the new file.
+  @param[in]   FileName    The Null-terminated string of the name of the file
+                           to be opened.
+  @param[in]   OpenMode    The mode to open the file : Read or Read/Write or
+                           Read/Write/Create
+  @param[in]   Attributes  Only valid for EFI_FILE_MODE_CREATE, in which case these
+                           are the attribute bits for the newly created file. The
+                           mnemonics of the attribute bits are : EFI_FILE_READ_ONLY,
+                           EFI_FILE_HIDDEN, EFI_FILE_SYSTEM, EFI_FILE_RESERVED,
+                           EFI_FILE_DIRECTORY and EFI_FILE_ARCHIVE.
+
+  @retval  EFI_SUCCESS            The file was open.
+  @retval  EFI_NOT_FOUND          The specified file could not be found.
+  @retval  EFI_DEVICE_ERROR       The last issued semi-hosting operation failed.
+  @retval  EFI_WRITE_PROTECTED    Attempt to create a directory. This is not possible
+                                  with the semi-hosting interface.
+  @retval  EFI_OUT_OF_RESOURCES   Not enough resources were available to open the file.
+  @retval  EFI_INVALID_PARAMETER  At least one of the parameters is invalid.
+
+**/
 EFI_STATUS
 FileOpen (
-  IN  EFI_FILE  *File,
+  IN  EFI_FILE  *This,
   OUT EFI_FILE  **NewHandle,
   IN  CHAR16    *FileName,
   IN  UINT64    OpenMode,