From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Brijesh Singh <brijesh.singh@amd.com>,
Jordan Justen <jordan.l.justen@intel.com>
Subject: [PATCH] OvmfPkg: fix dynamic default for oprom verification policy PCD without SB
Date: Wed, 18 Oct 2017 00:23:21 +0200 [thread overview]
Message-ID: <20171017222321.15381-1-lersek@redhat.com> (raw)
I missed the following, both while reviewing and while testing commit
6041ac65ae87 ("OvmfPkg/PlatformPei: DENY_EXECUTE_ON_SECURITY_VIOLATION
when SEV is active", 2017-10-05):
If "-D SECURE_BOOT_ENABLE" is not passed on the "build" command line, then
OVMF has no dynamic default at all for
"PcdOptionRomImageVerificationPolicy". This means that the PcdSet32S()
call added in the subject commit doesn't even compile:
> OvmfPkg/PlatformPei/AmdSev.c: In function 'AmdSevInitialize':
> OvmfPkg/PlatformPei/AmdSev.c:67:3: error: implicit declaration of
> function '_PCD_SET_MODE_32_S_PcdOptionRomImageVerificationPolicy'
> [-Werror=implicit-function-declaration]
> PcdStatus = PcdSet32S (PcdOptionRomImageVerificationPolicy, 0x4);
> ^
> cc1: all warnings being treated as errors
There are three ways to fix the error:
(1) Make the current, SB-only, 0x00 dynamic default unconditional.
This is the simplest approach, and it reflects the intent of original
commit 1fea9ddb4e3f ("OvmfPkg: execute option ROM images regardless of
Secure Boot", 2016-01-07). Without SECURE_BOOT_ENABLE,
"SecurityPkg/Library/DxeImageVerificationLib" is not used anyway, so
the PCD is never read.
(2) Add an !else branch that explicitly sets the SecurityPkg.dec default
(0x04) as dynamic default, if SECURE_BOOT_ENABLE is FALSE.
This looks awkward because it explicitly sets a dynamic default that
is then never read.
(3) Set the SecurityPkg.dec default (0x04) as unconditional dynamic
default, and invert the logic in AmdSevInitialize()
[OvmfPkg/PlatformPei/AmdSev.c] -- set the PCD to 0x00 if SEV is
disabled; don't touch it otherwise.
I think this sends the wrong message -- for the time being anyway, SEV
is the exception, not the rule. We shouldn't rely on the PCD getting
its most commonly used value in a function called AmdSevInitialize().
This issue was caught and reported by Gerd Hoffmann <kraxel@redhat.com>'s
Jenkins CI.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Fixes: 6041ac65ae879389f3ab5c0699f916d3e71c97fe
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
Repo: https://github.com/lersek/edk2.git
Branch: oprom_policy_build_fix
OvmfPkg/OvmfPkgIa32.dsc | 3 ---
OvmfPkg/OvmfPkgIa32X64.dsc | 3 ---
OvmfPkg/OvmfPkgX64.dsc | 3 ---
3 files changed, 9 deletions(-)
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 7fb557b7c9cd..c2f534fdbf3b 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -540,10 +540,7 @@ [PcdsDynamicDefault]
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000
!endif
-!if $(SECURE_BOOT_ENABLE) == TRUE
gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00
-!endif
-
################################################################################
#
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 4bcbddb95768..9f300a2e6f32 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -548,10 +548,7 @@ [PcdsDynamicDefault]
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000
!endif
-!if $(SECURE_BOOT_ENABLE) == TRUE
gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00
-!endif
-
################################################################################
#
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index e52a3bd4db9b..1ffcf37f8b92 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -547,10 +547,7 @@ [PcdsDynamicDefault]
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000
!endif
-!if $(SECURE_BOOT_ENABLE) == TRUE
gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00
-!endif
-
################################################################################
#
--
2.14.1.3.gb7cf6e02401b
next reply other threads:[~2017-10-17 22:19 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-17 22:23 Laszlo Ersek [this message]
2017-10-19 2:14 ` [PATCH] OvmfPkg: fix dynamic default for oprom verification policy PCD without SB Jordan Justen
2017-10-19 8:54 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171017222321.15381-1-lersek@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox