1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH v4 0/7] OvmfPkg: implement initrd shell command and mixed mode loader
  4. View patch

[edk2-devel] [PATCH v4 7/7] OvmfPkg/LinuxInitrdDynamicShellCommand: bail if initrd already exists

Ard Biesheuvel posted 7 patches 5 years, 11 months ago
[edk2-devel] [PATCH v4 7/7] OvmfPkg/LinuxInitrdDynamicShellCommand: bail if initrd already exists
Posted by Ard Biesheuvel 5 years, 11 months ago 
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.biesheuvel@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;
+}
+
 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"
-- 
2.17.1


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

View/Reply Online (#55335): https://edk2.groups.io/g/devel/message/55335
Mute This Topic: https://groups.io/mt/71700589/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v4 7/7] OvmfPkg/LinuxInitrdDynamicShellCommand: bail if initrd already exists
Posted by Laszlo Ersek 5 years, 11 months ago 
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.biesheuvel@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.

Reviewed-by: Laszlo Ersek <lersek@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 (#55344): https://edk2.groups.io/g/devel/message/55344
Mute This Topic: https://groups.io/mt/71700589/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v4 7/7] OvmfPkg/LinuxInitrdDynamicShellCommand: bail if initrd already exists
Posted by Laszlo Ersek 5 years, 11 months ago 
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.biesheuvel@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 <lersek@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v4 7/7] OvmfPkg/LinuxInitrdDynamicShellCommand: bail if initrd already exists
Posted by Ard Biesheuvel 5 years, 11 months ago 
On Tue, 3 Mar 2020 at 22:08, Laszlo Ersek <lersek@redhat.com> wrote:
>
> 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.biesheuvel@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.
>

Indeed

> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> >

Thanks!

I will push this series (asl well as the TPM one for ArmVirtPkg)
tomorrow, once the stable tag is assigned.


> >
> >
> >> +
> >>  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 (#55347): https://edk2.groups.io/g/devel/message/55347
Mute This Topic: https://groups.io/mt/71700589/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.