mbox series

[v3,0/6] SEV: add kernel-hashes=on for measured -kernel launch

Message ID 20211111100048.3299424-1-dovmurik@linux.ibm.com (mailing list archive)
Headers show
Series SEV: add kernel-hashes=on for measured -kernel launch | expand

Message

Dov Murik Nov. 11, 2021, 10 a.m. UTC
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 v3:
 - Patch 1/6: Add "(since 6.2)" in the documentation of the
   kernel-hashes option (thanks Markus)
 - Patch 3/6: Change error string use "kernel" instead of "-kernel"
   (thanks Daniel)

v2: https://lore.kernel.org/qemu-devel/20211108134840.2757206-1-dovmurik@linux.ibm.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

Comments

Dov Murik Nov. 14, 2021, 6:02 p.m. UTC | #1
Paolo,

Can you please add this series (already reviewed) to the fixes in 6.2?

Thanks,
-Dov


On 11/11/2021 12:00, 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://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 v3:
>  - Patch 1/6: Add "(since 6.2)" in the documentation of the
>    kernel-hashes option (thanks Markus)
>  - Patch 3/6: Change error string use "kernel" instead of "-kernel"
>    (thanks Daniel)
> 
> v2: https://lore.kernel.org/qemu-devel/20211108134840.2757206-1-dovmurik@linux.ibm.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
>
Dov Murik Nov. 18, 2021, 12:21 p.m. UTC | #2
Pinging again -- Daniel said this should be added to 6.2.

Is there anything I should do?

Thanks,
-Dov

On 14/11/2021 20:02, Dov Murik wrote:
> Paolo,
> 
> Can you please add this series (already reviewed) to the fixes in 6.2?
> 
> Thanks,
> -Dov
> 
> 
> On 11/11/2021 12:00, 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://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 v3:
>>  - Patch 1/6: Add "(since 6.2)" in the documentation of the
>>    kernel-hashes option (thanks Markus)
>>  - Patch 3/6: Change error string use "kernel" instead of "-kernel"
>>    (thanks Daniel)
>>
>> v2: https://lore.kernel.org/qemu-devel/20211108134840.2757206-1-dovmurik@linux.ibm.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
>>
Daniel P. Berrangé Nov. 18, 2021, 1:02 p.m. UTC | #3
On Thu, Nov 18, 2021 at 02:21:09PM +0200, Dov Murik wrote:
> Pinging again -- Daniel said this should be added to 6.2.
> 
> Is there anything I should do?

I'm going to take care of sending a PULL to relieve Paolo's
workload.


Regards,
Daniel
Dov Murik Nov. 18, 2021, 1:11 p.m. UTC | #4
On 18/11/2021 15:02, Daniel P. Berrangé wrote:
> On Thu, Nov 18, 2021 at 02:21:09PM +0200, Dov Murik wrote:
>> Pinging again -- Daniel said this should be added to 6.2.
>>
>> Is there anything I should do?
> 
> I'm going to take care of sending a PULL to relieve Paolo's
> workload.
> 

Thanks Daniel.

-Dov
Paolo Bonzini Nov. 18, 2021, 1:40 p.m. UTC | #5
On 11/18/21 14:02, Daniel P. Berrangé wrote:
> On Thu, Nov 18, 2021 at 02:21:09PM +0200, Dov Murik wrote:
>> Pinging again -- Daniel said this should be added to 6.2.
>>
>> Is there anything I should do?
> 
> I'm going to take care of sending a PULL to relieve Paolo's
> workload.

Apologies, I ignored the series last week because it sounded like 7.0 
material.

Paolo