[PULL,36/45] i386/sev: Invoke launch_updata_data() for SEV class

Commit Message

Paolo Bonzini June 4, 2024, 6:44 a.m. UTC

Add launch_update_data() in SevCommonStateClass and
invoke as sev_launch_update_data() for SEV object.

Signed-off-by: Pankaj Gupta <[e-mail]>
Message-ID: <[e-mail]>
Signed-off-by: Paolo Bonzini <[e-mail]>
---
 target/i386/sev.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Peter Maydell June 7, 2024, 2:18 p.m. UTC | #1

On Tue, 4 Jun 2024 at 07:49, Paolo Bonzini <[e-mail]> wrote:
>
> Add launch_update_data() in SevCommonStateClass and
> invoke as sev_launch_update_data() for SEV object.
>
> Signed-off-by: Pankaj Gupta <[e-mail]>
> Message-ID: <[e-mail]>
> Signed-off-by: Paolo Bonzini <[e-mail]>

Hi; Coverity points out an issue in this code (CID 1546886):

>  sev_encrypt_flash(hwaddr gpa, uint8_t *ptr, uint64_t len, Error **errp)
>  {
>      SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
> +    SevCommonStateClass *klass = SEV_COMMON_GET_CLASS(sev_common);

SEV_COMMON_GET_CLASS() dereferences the pointer it is passed,
so it isn't valid to pass it a NULL pointer...

>
>      if (!sev_common) {
>          return 0;

...but we don't do the "return failure if passed NULL" until after
we've dereferenced sev_common.

The get-the-class-pointer operation should be done after this
check, I think.

thanks
-- PMM

Patch

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 7b5c4b4874d..8834cf9441a 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -74,6 +74,7 @@  struct SevCommonStateClass {
     /* public */
     int (*launch_start)(SevCommonState *sev_common);
     void (*launch_finish)(SevCommonState *sev_common);
+    int (*launch_update_data)(SevCommonState *sev_common, hwaddr gpa, uint8_t *ptr, uint64_t len);
     int (*kvm_init)(ConfidentialGuestSupport *cgs, Error **errp);
 };
 
@@ -929,7 +930,7 @@  out:
 }
 
 static int
-sev_launch_update_data(SevGuestState *sev_guest, uint8_t *addr, uint64_t len)
+sev_launch_update_data(SevCommonState *sev_common, hwaddr gpa, uint8_t *addr, uint64_t len)
 {
     int ret, fw_error;
     struct kvm_sev_launch_update_data update;
@@ -941,7 +942,7 @@  sev_launch_update_data(SevGuestState *sev_guest, uint8_t *addr, uint64_t len)
     update.uaddr = (uintptr_t)addr;
     update.len = len;
     trace_kvm_sev_launch_update_data(addr, len);
-    ret = sev_ioctl(SEV_COMMON(sev_guest)->sev_fd, KVM_SEV_LAUNCH_UPDATE_DATA,
+    ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_LAUNCH_UPDATE_DATA,
                     &update, &fw_error);
     if (ret) {
         error_report("%s: LAUNCH_UPDATE ret=%d fw_error=%d '%s'",
@@ -1487,6 +1488,7 @@  int
 sev_encrypt_flash(hwaddr gpa, uint8_t *ptr, uint64_t len, Error **errp)
 {
     SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
+    SevCommonStateClass *klass = SEV_COMMON_GET_CLASS(sev_common);
 
     if (!sev_common) {
         return 0;
@@ -1494,7 +1496,9 @@  sev_encrypt_flash(hwaddr gpa, uint8_t *ptr, uint64_t len, Error **errp)
 
     /* if SEV is in update state then encrypt the data else do nothing */
     if (sev_check_state(sev_common, SEV_STATE_LAUNCH_UPDATE)) {
-        int ret = sev_launch_update_data(SEV_GUEST(sev_common), ptr, len);
+        int ret;
+
+        ret = klass->launch_update_data(sev_common, gpa, ptr, len);
         if (ret < 0) {
             error_setg(errp, "SEV: Failed to encrypt pflash rom");
             return ret;
@@ -1968,6 +1972,7 @@  sev_guest_class_init(ObjectClass *oc, void *data)
 
     klass->launch_start = sev_launch_start;
     klass->launch_finish = sev_launch_finish;
+    klass->launch_update_data = sev_launch_update_data;
     klass->kvm_init = sev_kvm_init;
     x86_klass->kvm_type = sev_kvm_type;