On 06/28/21 12:51, Dov Murik wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3457 > > In order to support measured SEV boot with kernel/initrd/cmdline, we'd > like to have one place that reads those blobs; in the future we'll add > the measurement and verification in that place. > > We already have a synthetic filesystem (QemuKernelLoaderFs) which holds > three files: "kernel", "initrd", and "cmdline". The kernel is indeed > read from this filesystem in LoadImage; but the cmdline (and the length > of initrd) are read from QemuFwCfgLib items. > > This patch series first fixes two identical memory leak bugs in > GenericQemuLoadImageLib and X86QemuLoadImageLib; then modifies > GenericQemuLoadImageLib to read cmdline (and the initrd size) from the > QemuKernelLoaderFs synthetic filesystem, thus removing the dependency on > QemuFwCfgLib. > > Note that X86QemuLoadImageLib is not modified, because it contains a > QemuLoadLegacyImage() which reads other items of the QemuFwCfg which are > not available in QemuKernelLoaderFs. Since we don't want to support the > legacy boot path in the future measured SEV boot, we leave > X86QemuLoadImageLib as-is (except for a comment addition in patch 3) and > will force use for GenericQemuLoadImageLib in the measured SEV boot > implementation. > > Relevant discussion threads start in: > https://edk2.groups.io/g/devel/message/76069 > > To test this on x86_64, I forced the use of GenericQemuLoadImageLib > using the following local patch: > > > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 0a237a905866..46442b543bcf 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -404,7 +404,7 @@ [LibraryClasses.common.DXE_DRIVER] > PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf > MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf > - > QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf > + > QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf > # XXX don't commit this or someone will be mad > !if $(TPM_ENABLE) == TRUE > Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf > Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf > > > I tested boot with QEMU and OVMF with the following QEMU arguments: > > -kernel a > -kernel a -initrd b > -kernel a -cmdline c > -kernel a -initrd b -cmdline c > > (and also without -kernel) > > > Code is at > https://github.com/confidential-containers-demo/edk2/tree/use-synthetic-fs-for-cmdline-v3 > > v3 changes: > - Insert patches 1+2 at the top of the series to fix cmdline leak bugs > - Organize #include and .inf > - Add UINTN overflow check > - Fix error paths and function epilogue to properly release all resources > - Clarity: rename long variables, reword comments > > v2: https://edk2.groups.io/g/devel/message/76664 > v2 changes: > - Add comment to header of X86QemuLoadImageLib.inf > - Clearer function names in GenericQemuLoadImageLib.c > - Fix coding style issues > > v1: https://edk2.groups.io/g/devel/message/76265 > > > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org> > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Cc: James Bottomley <j...@linux.ibm.com> > Cc: Tobin Feldman-Fitzthum <to...@linux.ibm.com> > > > Dov Murik (5): > OvmfPkg/GenericQemuLoadImageLib: plug cmdline blob leak on success > OvmfPkg/X86QemuLoadImageLib: plug cmdline blob leak on success > Revert "OvmfPkg/QemuKernelLoaderFsDxe: don't expose kernel command > line" > OvmfPkg/GenericQemuLoadImageLib: Read cmdline from QemuKernelLoaderFs > OvmfPkg/X86QemuLoadImageLib: State fw_cfg dependency in file header > > OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf | 3 +- > OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf | 3 + > OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.c | 157 > ++++++++++++++++++-- > OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c | 9 +- > OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c | 11 +- > 5 files changed, 161 insertions(+), 22 deletions(-) >
Merged as commit range d1fc3d7ef3cb..9421f5ab8d1e, via <https://github.com/tianocore/edk2/pull/1770>. (The BZ remains open for the upcoming (related) patch sets.) Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#77271): https://edk2.groups.io/g/devel/message/77271 Mute This Topic: https://groups.io/mt/83841915/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
