From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.2590.1685465463091969194 for ; Tue, 30 May 2023 09:51:03 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=CvwVe+4O; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: osde@linux.microsoft.com) Received: from [10.137.194.171] (unknown [131.107.1.171]) by linux.microsoft.com (Postfix) with ESMTPSA id 8931920FC3D0; Tue, 30 May 2023 09:51:02 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 8931920FC3D0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1685465462; bh=g562EGESYC6DfZ4ktGCA063hE0gwM8yOvQE+WaZ339c=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=CvwVe+4OKYllJ6P7V79RblLtiLm+i9EpzcKBf2grBS/DInM55+jovfDAwMCR1/Y8+ njKUZ3ntJee00RNHJDf5zsaxgV11Ukt4v+QRSd1m7c7h3XuqDWLdTY4hr4x04kb3ik /M3XxM4qnGD0qQmZSYz5r6HGg3aSwsmAxQPDYJN0= Message-ID: Date: Tue, 30 May 2023 09:51:02 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [edk2-devel] [RFC PATCH 07/10] MdeModulePkg/PeiCore: Apply restricted permissions in image loader To: Ard Biesheuvel Cc: devel@edk2.groups.io, Ray Ni , Jiewen Yao , Gerd Hoffmann , Taylor Beebe , Oliver Smith-Denny , Dandan Bi , Liming Gao , "Kinney, Michael D" , Leif Lindholm , Sunil V L , Andrei Warkentin References: <20230525143041.1172989-1-ardb@kernel.org> <20230525143041.1172989-8-ardb@kernel.org> From: "Oliver Smith-Denny" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 5/25/2023 2:29 PM, Ard Biesheuvel wrote: > On Thu, 25 May 2023 at 19:21, Oliver Smith-Denny > wrote: >> >> On 5/25/2023 7:30 AM, Ard Biesheuvel wrote: >>> + if (Private->MemoryAttributePpi == NULL) { >>> >>> + return EFI_SUCCESS; >> >> We will have a gap here before the MemoryAttributePpi is installed, >> obviously, when CpuPei is installed. Is the expectation that only >> dependencies for CpuPei will be launched before and everything else >> will have a dependency on CpuPei? >> >> Or, is it that shadowed PEIMs won't happen before CpuPei? I am curious >> how big or small of a gap this really is. >> > > There are two different cases to consider here: > - First, there is the DxeIpl, which will rely on the PPI (via the > image loader and directly) to map the DXE core and the NX stack. The > DxeIpl will not proceed with that until all PEIMs are dispatched, so > if the PPI is going to be exposed, it will be available by that time. > - Then there are the shadowed PEIMs, and given that shadowed PEIMs > implicitly depend on PEI permanent memory having been installed, the > only requirement here is that, if the platform needs/wants this for > shadowed PEIMs as well, the PEIM that installs the PEI permanent > memory should depex on the PPI. > I see, thanks for the explanation. While this is not the main point of this patchset, I do think that the core should own the memory protections of the shadowed PEIMs, instead of relying on the platform to do the right thing (which it never does :). Sure, this is gated by PCDs that the platform sets, but that is a simpler interface than requiring a new depex. Perhaps this is not trivial, as I assume the permanent memory installation happens in a platform specific PEIM? And as such would require a PPI notification in the core for the memory attributes PPI, which would lead to additional undesirable complexity. >>> >>> + } >>> >>> + >>> >>> + // >>> >>> + // PEI phase executables must be able to execute in place from read-only NOR >>> >>> + // flash, and so they can be mapped read-only in their entirety. >>> >>> + // >>> >>> + if ((FileType == EFI_FV_FILETYPE_PEI_CORE) || >>> >>> + (FileType == EFI_FV_FILETYPE_PEIM) || >>> >>> + (FileType == EFI_FV_FILETYPE_COMBINED_PEIM_DRIVER)) >>> >>> + { >> >> We are calling this from PEI Core, will we have more images of type >> PEI Core? I understand if this is for completeness, but just making >> sure I understand the flow. >> > > PEI core itself may be shadowed as well, and will be mapped read-only > as well if it is included here. > Gotcha, thanks. Oliver