From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, dovmurik@linux.ibm.com
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>,
Jordan Justen <jordan.l.justen@intel.com>,
James Bottomley <jejb@linux.ibm.com>,
Tobin Feldman-Fitzthum <tobin@linux.ibm.com>
Subject: Re: [edk2-devel] [PATCH v3 0/5] OvmfPkg: Use QemuKernelLoaderFs to read cmdline/initrd
Date: Tue, 29 Jun 2021 14:54:29 +0200 [thread overview]
Message-ID: <65d9fc4d-998f-1b94-4d79-1ff9df26c93f@redhat.com> (raw)
In-Reply-To: <20210628105110.379951-1-dovmurik@linux.ibm.com>
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 <lersek@redhat.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Tobin Feldman-Fitzthum <tobin@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
next prev parent reply other threads:[~2021-06-29 12:54 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-28 10:51 [PATCH v3 0/5] OvmfPkg: Use QemuKernelLoaderFs to read cmdline/initrd Dov Murik
2021-06-28 10:51 ` [PATCH v3 1/5] OvmfPkg/GenericQemuLoadImageLib: plug cmdline blob leak on success Dov Murik
2021-06-29 9:27 ` [edk2-devel] " Laszlo Ersek
2021-06-29 10:23 ` Laszlo Ersek
2021-06-28 10:51 ` [PATCH v3 2/5] OvmfPkg/X86QemuLoadImageLib: " Dov Murik
2021-06-29 9:31 ` [edk2-devel] " Laszlo Ersek
2021-06-29 11:18 ` Laszlo Ersek
2021-06-28 10:51 ` [PATCH v3 3/5] Revert "OvmfPkg/QemuKernelLoaderFsDxe: don't expose kernel command line" Dov Murik
2021-06-29 9:32 ` [edk2-devel] " Laszlo Ersek
2021-06-29 10:23 ` Laszlo Ersek
2021-06-28 10:51 ` [PATCH v3 4/5] OvmfPkg/GenericQemuLoadImageLib: Read cmdline from QemuKernelLoaderFs Dov Murik
2021-06-29 9:44 ` [edk2-devel] " Laszlo Ersek
2021-06-29 10:23 ` Laszlo Ersek
2021-06-28 10:51 ` [PATCH v3 5/5] OvmfPkg/X86QemuLoadImageLib: State fw_cfg dependency in file header Dov Murik
2021-06-29 9:45 ` [edk2-devel] " Laszlo Ersek
2021-06-29 12:54 ` Laszlo Ersek [this message]
2021-06-29 13:03 ` [edk2-devel] [PATCH v3 0/5] OvmfPkg: Use QemuKernelLoaderFs to read cmdline/initrd Dov Murik
2021-06-29 13:30 ` 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=65d9fc4d-998f-1b94-4d79-1ff9df26c93f@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