On 03/03/20 22:03, Laszlo Ersek wrote:
> On 03/03/20 15:01, Ard Biesheuvel wrote:
>> Before taking any actions, check if an instance of the LoadFile2 exists
>> already on the Linux initrd media GUID device path, and whether it was
>> provided by this command. If so, abort, since no duplicate instances of
>> the device path should exist.
>>
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2564
>> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
>> ---
>>  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.c   | 
>> 31 ++++++++++++++++++++
>>  OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.uni | 
>>  3 ++
>>  2 files changed, 34 insertions(+)
>>
>> diff --git 
>> a/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.c 
>> b/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.c
>> index 47ed26b50d3a..ed8fbaa77069 100644
>> --- a/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.c
>> +++ b/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.c
>> @@ -53,6 +53,33 @@ STATIC CONST SINGLE_NODE_VENDOR_MEDIA_DEVPATH 
>> mInitrdDevicePath = {
>>    }
>>  };
>>  
>> +STATIC
>> +BOOLEAN
>> +IsOtherInitrdDevicePathAlreadyInstalled (
>> +  VOID
>> +  )
>> +{
>> +  EFI_STATUS                  Status;
>> +  EFI_DEVICE_PATH_PROTOCOL    *DevicePath;
>> +  EFI_HANDLE                  Handle;
>> +
>> +  DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)&mInitrdDevicePath;
>> +  Status = gBS->LocateDevicePath (&gEfiLoadFile2ProtocolGuid, &DevicePath,
>> +                  &Handle);
>> +  if (EFI_ERROR (Status)) {
>> +    return FALSE;
>> +  }
>> +
>> +  //
>> +  // Check whether the existing instance is one that we installed during
>> +  // a previous invocation.
>> +  //
>> +  if (Handle == mInitrdLoadFile2Handle) {
>> +    return FALSE;
>> +  }
>> +  return TRUE;
>> +}
> 
> Looks good, the function will return TRUE when mInitrdLoadFile2Handle is
> NULL.

To clarify, what I mean is that the controlling expression

  (Handle == mInitrdLoadFile2Handle)

assuming it is reached, will evaluate to 0, if mInitrdLoadFile2Handle is
NULL. That's because Handle cannot be NULL at that point. Hence the
function will return TRUE.

Thanks
Laszlo

> Reviewed-by: Laszlo Ersek <ler...@redhat.com>
> 
> Thanks
> Laszlo
> 
> 
>> +
>>  STATIC
>>  EFI_STATUS
>>  EFIAPI
>> @@ -217,6 +244,10 @@ RunInitrd (
>>      } else {
>>        ASSERT(FALSE);
>>      }
>> +  } else if (IsOtherInitrdDevicePathAlreadyInstalled ()) {
>> +    ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_ALREADY_INSTALLED),
>> +      mLinuxInitrdShellCommandHiiHandle, L"initrd");
>> +    ShellStatus = SHELL_UNSUPPORTED;
>>    } else {
>>      if (ShellCommandLineGetCount (Package) > 2) {
>>        ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY),
>> diff --git 
>> a/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.uni 
>> b/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.uni
>> index a88fa6e3641b..4b6b1285fffd 100644
>> --- 
>> a/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.uni
>> +++ 
>> b/OvmfPkg/LinuxInitrdDynamicShellCommand/LinuxInitrdDynamicShellCommand.uni
>> @@ -18,6 +18,7 @@
>>  #langdef   en-US "english"
>>  
>>  #string STR_GEN_PROBLEM           #language en-US "%H%s%N: Unknown flag - 
>> '%H%s%N'\r\n"
>> +#string STR_GEN_ALREADY_INSTALLED #language en-US "%H%s%N: Linux initrd 
>> already provided by platform\r\n"
>>  #string STR_GEN_TOO_MANY          #language en-US "%H%s%N: Too many 
>> arguments.\r\n"
>>  #string STR_GEN_TOO_FEW           #language en-US "%H%s%N: Too few 
>> arguments.\r\n"
>>  #string STR_GEN_FIND_FAIL         #language en-US "%H%s%N: File not found - 
>> '%H%s%N'\r\n"
>> @@ -47,3 +48,5 @@
>>  "     Consumers of the LoadFile2 protocol on the 
>> LINUX_EFI_INITRD_MEDIA_GUID\r\n"
>>  "     device path that are started via means other than the shell will be 
>> able\r\n"
>>  "     to locate the protocol and invoke it.\r\n"
>> +"  3. Exposing an initrd using this command is only supported if no initrd 
>> is\r\n"
>> +"     already being exposed by another driver on the platform.\r\n"
>>
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#55345): https://edk2.groups.io/g/devel/message/55345
Mute This Topic: https://groups.io/mt/71700589/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to