On 2/15/24 11:25, Gerd Hoffmann wrote: > The EFI Shell allows to bypass secure boot, do not allow > to include the shell in the firmware images of secure boot > enabled builds. > > This prevents misconfigured downstream builds. > > Ref: https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/2040137 > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4641 > Signed-off-by: Gerd Hoffmann <kra...@redhat.com> > --- > OvmfPkg/Include/Fdf/ShellDxe.fdf.inc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc > b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc > index 3081ac41780a..38f69747b02c 100644 > --- a/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc > +++ b/OvmfPkg/Include/Fdf/ShellDxe.fdf.inc > @@ -2,7 +2,7 @@ > # SPDX-License-Identifier: BSD-2-Clause-Patent > ## > > -!if $(BUILD_SHELL) == TRUE > +!if $(BUILD_SHELL) == TRUE && $(SECURE_BOOT_ENABLE) == FALSE > > !if $(TOOL_CHAIN_TAG) != "XCODE5" > !if $(NETWORK_ENABLE) == TRUE
This does the job: Reviewed-by: Laszlo Ersek <ler...@redhat.com> An alternative could be (perhaps informing the user better): !if $(BUILD_SHELL) == TRUE !if $(SECURE_BOOT_ENABLE) == TRUE !error BUILD_SHELL and SECURE_BOOT_ENABLE conflict !endif ... !endif If you prefer that approach, feel free to submit a v3 (you can add my R-b for that variant immediately). A disadvantage might be that, with such a patch in place, people having built OVMF with "-D SECURE_BOOT_ENABLE" thus far would start getting build failures, and they'd have to explicitly pass "-D BUILD_SHELL=FALSE", IIUC. (Well, some might consider such a build failure an advantage actually: near the BUILD_SHELL define, we have a comment saying "Shell can be useful for debugging but should not be enabled for production". Pick your poison, I guess...) A repost might be worth your while either way, because some of the patches are identical to their first versions, and Jiewen's v1 Acked-by, from [1], is missing from the unchanged (or trivially rebased) patches in v2. I strongly prefer capturing all feedback tags in the git history; after all, that keeps a record of reviewer *work* (not just the mere approval itself). [1] https://edk2.groups.io/g/devel/message/114362 msgid <mw4pr11mb5872132f77f29dc6758c71178c...@mw4pr11mb5872.namprd11.prod.outlook.com> Thanks, Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115551): https://edk2.groups.io/g/devel/message/115551 Mute This Topic: https://groups.io/mt/104370218/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-