From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web10.47.1685479891966747023 for ; Tue, 30 May 2023 13:51:32 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=mLwRH/Ey; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 53726602E8 for ; Tue, 30 May 2023 20:51:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BBC1BC4339C for ; Tue, 30 May 2023 20:51:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685479890; bh=isKJqgcL036SVnz/gVOtdPhDi9iz62OsWUaaUOIM8FQ=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=mLwRH/Eyh8Gylz5Osjmptm7g0OGXQRwLMVwuztA2jvULQ54C4jFmSoEgbRl6DqlQm YpAusUCSHLPBXrs1RKcrQz9GLocgxc3fYr2K3Clhp0t6t7z6N9lN89EraTzjrq2bkp H+iY0u0FDmZSzWXd2sUIOCYibMhcY3P30lT8294hVQlVD6I5C0D2s6rM0uAGfoRO4z 6HuzEEzedAcw8hd37oER7t+x3CgVNvYbeWa663Kywgfep2Ey1Y8UjB+zXiuGKozpvu p4Cy5EgjIIJXnxer7iFBKfPohEwJ93l+hjGs8KRUx1JQu21kRVDXxs6USzdm1d3lWP pQIuaEd5q3Rtg== Received: by mail-lj1-f173.google.com with SMTP id 38308e7fff4ca-2af2d092d7aso53564291fa.2 for ; Tue, 30 May 2023 13:51:30 -0700 (PDT) X-Gm-Message-State: AC+VfDzq6Sp9SRgKRftnQQsRvH3Oxw/EOnh0SAtiaho031aihkDs5xDu sYUUA1Cj7Skl/Au9krIl/S9Ng+TROONsE6DI1oE= X-Google-Smtp-Source: ACHHUZ4u2Jl55Y38898zLnYJpbd2N3PnRZsf+zdcZeCjyEwY251kqxEyl+kGPR4QDEGPZErL8aY/DxSp32gudD7Frys= X-Received: by 2002:a2e:a0d1:0:b0:2ad:95dd:8802 with SMTP id f17-20020a2ea0d1000000b002ad95dd8802mr1479012ljm.38.1685479888795; Tue, 30 May 2023 13:51:28 -0700 (PDT) MIME-Version: 1.0 References: <20230525143041.1172989-1-ardb@kernel.org> <20230525143041.1172989-8-ardb@kernel.org> In-Reply-To: From: "Ard Biesheuvel" Date: Tue, 30 May 2023 22:51:17 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [RFC PATCH 07/10] MdeModulePkg/PeiCore: Apply restricted permissions in image loader To: devel@edk2.groups.io, osde@linux.microsoft.com Cc: 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 Content-Type: text/plain; charset="UTF-8" On Tue, 30 May 2023 at 18:51, Oliver Smith-Denny wrote: > > 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. > This is simply a matter of policy. If we decide that shadowed PEIMs must always be remapped read-only, we should reorganize the code to ensure that, and disallow shadowing otherwise. Shadowing is a performance optimization in any case, although existing code may not work, due to the way RegisterForShadow() is designed. But this is all fixable. But that doesn't change the fact that installing PEI permanent memory is fundamentally something that is implemented in platform code. Each platform that already implements this should be able to take advantage of those core changes once they land.