From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by mx.groups.io with SMTP id smtpd.web11.1141.1583272395060956277 for ; Tue, 03 Mar 2020 13:53:15 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=xuy+TS3x; spf=pass (domain: linaro.org, ip: 209.85.221.66, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f66.google.com with SMTP id x7so6433334wrr.0 for ; Tue, 03 Mar 2020 13:53:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=qRx1rD/BnEbPT4Vc5yM3K9Wb9F+sTGrQTLA+4bySBR4=; b=xuy+TS3xRHNZcoB0qD40Vsm8V/FdassKupYyTNl5Tsa74GbVWmOJbu/Sihrds1JDNy boasGOEVTRzsVoF2uTAmxtEMWxCST3w5U3bThnUOYmnUwbtXBZjcc+/V6ZDrk/BfoIMB xqBpNATL7PraIVnt8Z6GsHSs2lwpITxHZ3oxBOrE7Kk9cyzjYd6nwP6PmxgVF6cFkBJi EFyhf6/8gLWi3eBUwdnl/bcU39RxL4mnhxV1szs1/jM9TeIIc1ibe0tGm1bZaJkeozl5 kSMH+8sZkm22R1Sa6LEw9D4KuZYG1nRgC1l+tYSN0fySCv2WyxFV6Ww+TTsMgqwAkRim xfnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=qRx1rD/BnEbPT4Vc5yM3K9Wb9F+sTGrQTLA+4bySBR4=; b=sFVQ4qd2NuvN1aDYMYa+e3xnYwbzKGw7DUi9nnahcG/txeZ49troLBzeI0j8Rw+dRL r5Kc2G88Ga64Nti8ivZkD9JxwHlAOOlZ7lYwCnVxdFyjyX8iWD7bpZFCgI5mbBgIkdwb UCP1Ewl8qAUc8HDxNKU3SwDMEzfOqXnPKxfDU9lic6R+45pj54/goCEgtkmLHfFmqjAi 3FV9Ree1HV9xDDjZ+NQLQWcjJYfI+huvOsHq8EXv68uVy+qWOZh80tF4E4iTgJW5WwCt JfyCyPLlkobBXVSoiM6OqFObzPVOZTn1oYkgTfYi62k3gjgJxdk5819IVsJOUF0ABOl4 uLJg== X-Gm-Message-State: ANhLgQ1Yx31+Tv0A9QFmSq6ReegDV1gXHJnyK1uqEGHfhDcQbVc0vEbY YzI0zW1bql5ftlk9ab94KGaMf06wV4d4go02STdlf+Gn0S0= X-Google-Smtp-Source: ADFU+vsoI6I3x2IP6RkfUq6TplIeAXlL2i7rSXDwLbXxNDRlVyt6N4C2QDHxlUFTU6jovZxIsJjCKBOfSoMkwAdBcJ8= X-Received: by 2002:a05:6000:110b:: with SMTP id z11mr103156wrw.252.1583272393332; Tue, 03 Mar 2020 13:53:13 -0800 (PST) MIME-Version: 1.0 References: <20200303140117.7288-1-ard.biesheuvel@linaro.org> <20200303140117.7288-8-ard.biesheuvel@linaro.org> <3f96cac2-06fe-9c5c-52d1-8da5f7096fb8@redhat.com> <04d4423d-b64b-9af3-b6e8-5b22731445da@redhat.com> In-Reply-To: <04d4423d-b64b-9af3-b6e8-5b22731445da@redhat.com> From: "Ard Biesheuvel" Date: Tue, 3 Mar 2020 22:53:02 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH v4 7/7] OvmfPkg/LinuxInitrdDynamicShellCommand: bail if initrd already exists To: edk2-devel-groups-io , Laszlo Ersek Cc: Leif Lindholm , Liming Gao Content-Type: text/plain; charset="UTF-8" On Tue, 3 Mar 2020 at 22:08, Laszlo Ersek 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 > >> --- > >> 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 > > 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" > >> > > > > > >