| Message ID | 20211108134840.2757206-1-dovmurik@linux.ibm.com |
|---|---|
| Headers | show |
| Series | SEV: add kernel-hashes=on for measured -kernel launch | expand |
On 11/8/21 7:48 AM, Dov Murik wrote: > Tom Lendacky and Brijesh Singh reported two issues with launching SEV > guests with the -kernel QEMU option when an old [1] or wrongly configured [2] > OVMF images are used. > > To fix these issues, these series "hides" the whole kernel hashes > additions behind a kernel-hashes=on option (with default value of > "off"). This allows existing scenarios to work without change, and > explicitly forces kernel hashes additions for guests that require that. > > Patch 1 introduces a new boolean option "kernel-hashes" on the sev-guest > object, and patch 2 causes QEMU to add kernel hashes only if its > explicitly set to "on". This will mitigate both experienced issues > because the default of the new setting is off, and therefore is backward > compatible with older OVMF images (which don't have a designated hashes > table area) or with guests that don't wish to measure the kernel/initrd. > > Patch 3 fixes the wording on the error message displayed when no hashes > table is found in the guest firmware. > > Patch 4 detects incorrect address and length of the guest firmware > hashes table area and fails the boot. > > Patch 5 is a refactoring of parts of the same function > sev_add_kernel_loader_hashes() to calculate all padding sizes at > compile-time. Patch 6 also changes the same function and replaces the > call to qemu_map_ram_ptr() with address_space_map() to allow for error > detection. Patches 5-6 are not required to fix the issues above, but > are suggested as an improvement (no functional change intended). > > To enable addition of kernel/initrd/cmdline hashes into the SEV guest at > launch time, specify: > > qemu-system-x86_64 ... -object sev-guest,...,kernel-hashes=on > > > [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fqemu-devel%2F3b9d10d9-5d9c-da52-f18c-cd93c1931706%40amd.com%2F&data=04%7C01%7Cbrijesh.singh%40amd.com%7C908b739400a747e1b22308d9a2be7e07%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637719761315906327%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=cMoOlNU2faGwRk6dXVmOE1SuNrg3VvySAC1Ds8fcaFQ%3D&reserved=0 > [2] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fqemu-devel%2F001dd81a-282d-c307-a657-e228480d4af3%40amd.com%2F&data=04%7C01%7Cbrijesh.singh%40amd.com%7C908b739400a747e1b22308d9a2be7e07%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637719761315916323%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=7IZ1%2B%2Fh%2B88xWDlHd%2FMKPN0fJfI6dmSX%2F1TbK8aL8bAs%3D&reserved=0 > > > > Changes in v2: > - Instead of trying to figure out whether to add hashes or not, > explicity declare an option (kernel-hashes=on) for that. When that > option is turned on, fail if the hashes cannot be added. > - Rephrase error message when no hashes table GUID is found. > - Replace qemu_map_ram_ptr with address_space_map > > v1: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fqemu-devel%2F20211101102136.1706421-1-dovmurik%40linux.ibm.com%2F&data=04%7C01%7Cbrijesh.singh%40amd.com%7C908b739400a747e1b22308d9a2be7e07%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637719761315916323%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=SrE9kYP0Qdhx0WqIXbnwHgeX%2BjBVT9BsK6I0OLU3naI%3D&reserved=0 > > > Dov Murik (6): > qapi/qom,target/i386: sev-guest: Introduce kernel-hashes=on|off option > target/i386/sev: Add kernel hashes only if sev-guest.kernel-hashes=on > target/i386/sev: Rephrase error message when no hashes table in guest > firmware > target/i386/sev: Fail when invalid hashes table area detected > target/i386/sev: Perform padding calculations at compile-time > target/i386/sev: Replace qemu_map_ram_ptr with address_space_map > > qapi/qom.json | 7 ++++- > target/i386/sev.c | 77 +++++++++++++++++++++++++++++++++++++++-------- > qemu-options.hx | 6 +++- > 3 files changed, 75 insertions(+), 15 deletions(-) > > Thanks for the fixing it Dov. Acked-by: Brijesh Singh <brijesh.singh@amd.com> thanks
On Mon, Nov 08, 2021 at 01:48:34PM +0000, Dov Murik wrote: > Tom Lendacky and Brijesh Singh reported two issues with launching SEV > guests with the -kernel QEMU option when an old [1] or wrongly configured [2] > OVMF images are used. > > To fix these issues, these series "hides" the whole kernel hashes > additions behind a kernel-hashes=on option (with default value of > "off"). This allows existing scenarios to work without change, and > explicitly forces kernel hashes additions for guests that require that. We need to to get this into 6.2 to adress the regression vs previous QEMU releases. There's just a couple of small review points. If you can repost with the easy fixes, then we can get this queued. Regards, Daniel
On 11/11/2021 11:39, Daniel P. Berrangé wrote: > On Mon, Nov 08, 2021 at 01:48:34PM +0000, Dov Murik wrote: >> Tom Lendacky and Brijesh Singh reported two issues with launching SEV >> guests with the -kernel QEMU option when an old [1] or wrongly configured [2] >> OVMF images are used. >> >> To fix these issues, these series "hides" the whole kernel hashes >> additions behind a kernel-hashes=on option (with default value of >> "off"). This allows existing scenarios to work without change, and >> explicitly forces kernel hashes additions for guests that require that. > > We need to to get this into 6.2 to adress the regression vs previous > QEMU releases. > > There's just a couple of small review points. If you can repost > with the easy fixes, then we can get this queued. > Sent v3 now. Patch 3/6 (error message rephrase) still misses Reviewed-by. Thanks, -Dov
Tom Lendacky and Brijesh Singh reported two issues with launching SEV guests with the -kernel QEMU option when an old [1] or wrongly configured [2] OVMF images are used. To fix these issues, these series "hides" the whole kernel hashes additions behind a kernel-hashes=on option (with default value of "off"). This allows existing scenarios to work without change, and explicitly forces kernel hashes additions for guests that require that. Patch 1 introduces a new boolean option "kernel-hashes" on the sev-guest object, and patch 2 causes QEMU to add kernel hashes only if its explicitly set to "on". This will mitigate both experienced issues because the default of the new setting is off, and therefore is backward compatible with older OVMF images (which don't have a designated hashes table area) or with guests that don't wish to measure the kernel/initrd. Patch 3 fixes the wording on the error message displayed when no hashes table is found in the guest firmware. Patch 4 detects incorrect address and length of the guest firmware hashes table area and fails the boot. Patch 5 is a refactoring of parts of the same function sev_add_kernel_loader_hashes() to calculate all padding sizes at compile-time. Patch 6 also changes the same function and replaces the call to qemu_map_ram_ptr() with address_space_map() to allow for error detection. Patches 5-6 are not required to fix the issues above, but are suggested as an improvement (no functional change intended). To enable addition of kernel/initrd/cmdline hashes into the SEV guest at launch time, specify: qemu-system-x86_64 ... -object sev-guest,...,kernel-hashes=on [1] https://lore.kernel.org/qemu-devel/3b9d10d9-5d9c-da52-f18c-cd93c1931706@amd.com/ [2] https://lore.kernel.org/qemu-devel/001dd81a-282d-c307-a657-e228480d4af3@amd.com/ Changes in v2: - Instead of trying to figure out whether to add hashes or not, explicity declare an option (kernel-hashes=on) for that. When that option is turned on, fail if the hashes cannot be added. - Rephrase error message when no hashes table GUID is found. - Replace qemu_map_ram_ptr with address_space_map v1: https://lore.kernel.org/qemu-devel/20211101102136.1706421-1-dovmurik@linux.ibm.com/ Dov Murik (6): qapi/qom,target/i386: sev-guest: Introduce kernel-hashes=on|off option target/i386/sev: Add kernel hashes only if sev-guest.kernel-hashes=on target/i386/sev: Rephrase error message when no hashes table in guest firmware target/i386/sev: Fail when invalid hashes table area detected target/i386/sev: Perform padding calculations at compile-time target/i386/sev: Replace qemu_map_ram_ptr with address_space_map qapi/qom.json | 7 ++++- target/i386/sev.c | 77 +++++++++++++++++++++++++++++++++++++++-------- qemu-options.hx | 6 +++- 3 files changed, 75 insertions(+), 15 deletions(-) base-commit: af531756d25541a1b3b3d9a14e72e7fedd941a2e