[PATCH 1/2] i386: assert 'cs->kvm_state' is not null

[PATCH 1/2] i386: assert 'cs->kvm_state' is not null

[Vitaly Kuznetsov]

Coverity reports potential NULL pointer dereference in
get_supported_hv_cpuid_legacy() when 'cs->kvm_state' is NULL. While
'cs->kvm_state' can indeed be NULL in hv_cpuid_get_host(),
kvm_hyperv_expand_features() makes sure that it only happens when
KVM_CAP_SYS_HYPERV_CPUID is supported and KVM_CAP_SYS_HYPERV_CPUID
implies KVM_CAP_HYPERV_CPUID so get_supported_hv_cpuid_legacy() is
never really called. Add asserts to strengthen the protection against
broken KVM behavior.

Coverity: CID 1458243
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 target/i386/kvm/kvm.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 59ed8327ac13..e69abe48e3f8 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -974,6 +974,12 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs)
     do_sys_ioctl =
         kvm_check_extension(kvm_state, KVM_CAP_SYS_HYPERV_CPUID) > 0;
 
+    /*
+     * Non-empty KVM context is needed when KVM_CAP_SYS_HYPERV_CPUID is
+     * unsupported, kvm_hyperv_expand_features() checks for that.
+     */
+    assert(do_sys_ioctl || cs->kvm_state);
+
     /*
      * When the buffer is too small, KVM_GET_SUPPORTED_HV_CPUID fails with
      * -E2BIG, however, it doesn't report back the right size. Keep increasing
@@ -1105,6 +1111,14 @@ static uint32_t hv_cpuid_get_host(CPUState *cs, uint32_t func, int reg)
         if (kvm_check_extension(kvm_state, KVM_CAP_HYPERV_CPUID) > 0) {
             cpuid = get_supported_hv_cpuid(cs);
         } else {
+            /*
+             * 'cs->kvm_state' may be NULL when Hyper-V features are expanded
+             * before KVM context is created but this is only done when
+             * KVM_CAP_SYS_HYPERV_CPUID is supported and it implies
+             * KVM_CAP_HYPERV_CPUID.
+             */
+            assert(cs->kvm_state);
+
             cpuid = get_supported_hv_cpuid_legacy(cs);
         }
         hv_cpuid_cache = cpuid;
-- 
2.31.1

[PATCH 2/2] i386: Fix coding style in kvm_hyperv_expand_features()

[Vitaly Kuznetsov]

QEMU coding style requires braces around bodies of ifs.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 target/i386/kvm/kvm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index e69abe48e3f8..28ca682b1089 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -1219,8 +1219,9 @@ bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp)
     Error *local_err = NULL;
     int feat;
 
-    if (!hyperv_enabled(cpu))
+    if (!hyperv_enabled(cpu)) {
         return true;
+    }
 
     /*
      * When kvm_hyperv_expand_features is called at CPU feature expansion
@@ -1228,8 +1229,9 @@ bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp)
      * when KVM_CAP_SYS_HYPERV_CPUID is supported.
      */
     if (!cs->kvm_state &&
-        !kvm_check_extension(kvm_state, KVM_CAP_SYS_HYPERV_CPUID))
+        !kvm_check_extension(kvm_state, KVM_CAP_SYS_HYPERV_CPUID)) {
         return true;
+    }
 
     if (cpu->hyperv_passthrough) {
         cpu->hyperv_vendor_id[0] =
-- 
2.31.1

Re: [PATCH 2/2] i386: Fix coding style in kvm_hyperv_expand_features()

[Paolo Bonzini]

On 16/07/21 13:58, Vitaly Kuznetsov wrote:
> QEMU coding style requires braces around bodies of ifs.
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>   target/i386/kvm/kvm.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index e69abe48e3f8..28ca682b1089 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -1219,8 +1219,9 @@ bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp)
>       Error *local_err = NULL;
>       int feat;
>   
> -    if (!hyperv_enabled(cpu))
> +    if (!hyperv_enabled(cpu)) {
>           return true;
> +    }
>   
>       /*
>        * When kvm_hyperv_expand_features is called at CPU feature expansion
> @@ -1228,8 +1229,9 @@ bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp)
>        * when KVM_CAP_SYS_HYPERV_CPUID is supported.
>        */
>       if (!cs->kvm_state &&
> -        !kvm_check_extension(kvm_state, KVM_CAP_SYS_HYPERV_CPUID))
> +        !kvm_check_extension(kvm_state, KVM_CAP_SYS_HYPERV_CPUID)) {
>           return true;
> +    }
>   
>       if (cpu->hyperv_passthrough) {
>           cpu->hyperv_vendor_id[0] =
> 

Queued both, thanks.

Paolo