From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web12.567.1583269719388408024 for ; Tue, 03 Mar 2020 13:08:39 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=bhdm6sZX; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1583269718; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=S3uT9ueglorw4gUfcSGFBzae5siabF9ODf/MLthe/sc=; b=bhdm6sZXUqCeCfAmlCOqj3zBXpDHGIFxVeECcGwPdE2/g4Yth+7IuY4eIF9Y6ozpHstRrm 23N8k8WcSqU6k4P8gJdRLatFNHshxhPiMGRAdiVCx1EKDuFsB7auMTaf+X1PYsSmJ7m+/l WVIKXWiQbEwyEpBN+t9kHG6NTYU92Sw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-311-TihJN2SuMqW88LWT7WHlzg-1; Tue, 03 Mar 2020 16:08:36 -0500 X-MC-Unique: TihJN2SuMqW88LWT7WHlzg-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A438B7A4B3; Tue, 3 Mar 2020 21:08:35 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-34.ams2.redhat.com [10.36.117.34]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7EC198CCE2; Tue, 3 Mar 2020 21:08:34 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v4 7/7] OvmfPkg/LinuxInitrdDynamicShellCommand: bail if initrd already exists From: "Laszlo Ersek" To: devel@edk2.groups.io, ard.biesheuvel@linaro.org Cc: leif@nuviainc.com, Liming Gao References: <20200303140117.7288-1-ard.biesheuvel@linaro.org> <20200303140117.7288-8-ard.biesheuvel@linaro.org> <3f96cac2-06fe-9c5c-52d1-8da5f7096fb8@redhat.com> Message-ID: <04d4423d-b64b-9af3-b6e8-5b22731445da@redhat.com> Date: Tue, 3 Mar 2020 22:08:33 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <3f96cac2-06fe-9c5c-52d1-8da5f7096fb8@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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. Thanks Laszlo > Reviewed-by: Laszlo Ersek > > 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" >> >