Re: [PATCH] MdePkg/UefiFileHandleLib: Tolerate more Root handle FileNames

["Liming Gao" <liming.gao@intel.com>]

Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: Gao, Zhichao
>Sent: Thursday, October 24, 2019 10:52 AM
>To: devel@edk2.groups.io; Marvin.Haeuser@outlook.com
>Cc: vit9696@protonmail.com; Kinney, Michael D
><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
>Subject: RE: [PATCH] MdePkg/UefiFileHandleLib: Tolerate more Root handle
>FileNames
>
>This patch makes sense. The patch would not affect the original logic and
>figure out the failure if the root directory file name is '\\'.
>Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
>
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Marvin Häuser
>> Sent: Sunday, October 20, 2019 8:09 PM
>> To: devel@edk2.groups.io
>> Cc: vit9696@protonmail.com; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
>> Subject: [edk2-devel] [PATCH] MdePkg/UefiFileHandleLib: Tolerate more
>> Root handle FileNames
>>
>> From: Marvin Haeuser <mhaeuser@outlook.de>
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2295
>>
>> The current implementation of the FileHandleGetFileName() function
>> assumes that the Root directory always has the FileName '\0'.
>> However, the only requirement the UEFI specification defines is that a
>> prepended '\\' must be supported to access files and folders relative to the
>> Root directory.
>> This patch removes this assumption and supports constructing valid paths
>for
>> any value of FileName for the Root Directory.
>>
>> In practice, this fixes compatibility issues with File System drivers that report
>> '\\' as the FileName of the Root directory, which currently is both generating
>> an invalid path ("\\\\") and resulting in an EFI_NOT_FOUND result from the
>> CurrentHandle->Open() call.
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
>> ---
>>  MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 25
>> ++++++++++++++------
>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>> b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>> index 5dc893833a46..28e28e5f67d5 100644
>> --- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>> +++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>> @@ -816,10 +816,25 @@ FileHandleGetFileName (
>>          Status = EFI_OUT_OF_RESOURCES;
>>          break;
>>        } else {
>> +        //
>> +        // Prepare to move to the parent directory.
>> +        // Also determine whether CurrentHandle refers to the Root directory.
>> +        //
>> +        Status = CurrentHandle->Open (CurrentHandle, &NextHigherHandle,
>> + L"..", EFI_FILE_MODE_READ, 0);
>>          //
>>          // We got info... do we have a name? if yes precede the current path
>> with it...
>>          //
>> -        if (StrLen (FileInfo->FileName) == 0) {
>> +        if ((StrLen (FileInfo->FileName) == 0) || EFI_ERROR (Status)) {
>> +          //
>> +          // Both FileInfo->FileName being '\0' and EFI_ERROR() suggest that
>> +          // CurrentHandle refers to the Root directory.  As this loop ensures
>> +          // FullFileName is starting with '\\' at all times, signal success
>> +          // and exit the loop.
>> +          // While FileInfo->FileName could theoretically be a value other than
>> +          // '\0' or '\\', '\\' is guaranteed to be supported by the
>> +          // specification and hence its value can safely be ignored.
>> +          //
>> +          Status = EFI_SUCCESS;
>>            if (*FullFileName == NULL) {
>>              ASSERT((*FullFileName == NULL && Size == 0) || (*FullFileName !=
>> NULL));
>>              *FullFileName = StrnCatGrowLeft(FullFileName, &Size, L"\\", 0); @@ -
>> 837,15 +852,11 @@ FileHandleGetFileName (
>>            FreePool(FileInfo);
>>          }
>>        }
>> +
>> +      FileHandleClose(CurrentHandle);
>>        //
>>        // Move to the parent directory
>>        //
>> -      Status = CurrentHandle->Open (CurrentHandle, &NextHigherHandle,
>> L"..", EFI_FILE_MODE_READ, 0);
>> -      if (EFI_ERROR (Status)) {
>> -        break;
>> -      }
>> -
>> -      FileHandleClose(CurrentHandle);
>>        CurrentHandle = NextHigherHandle;
>>      }
>>    } else if (Status == EFI_NOT_FOUND) {
>> --
>> 2.23.0.windows.1
>>
>>
>>