Re: [PATCH v3 1/4] KVM: x86: Fix a spurious -E2BIG in KVM_GET_EMULATED_CPUID

On Tue, Apr 06, 2021, Vitaly Kuznetsov wrote:

Emanuele Giuseppe Esposito eesposit@redhat.com writes:

...

When retrieving emulated CPUID entries, check for an insufficient array size if and only if KVM is actually inserting an entry. If userspace has a priori knowledge of the exact array size, KVM_GET_EMULATED_CPUID will incorrectly fail due to effectively requiring an extra, unused entry.

Signed-off-by: Emanuele Giuseppe Esposito eesposit@redhat.com

arch/x86/kvm/cpuid.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 6bd2f8b830e4..27059ddf9f0a 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -567,34 +567,33 @@ static struct kvm_cpuid_entry2 *do_host_cpuid(struct kvm_cpuid_array *array, static int __do_cpuid_func_emulated(struct kvm_cpuid_array *array, u32 func) {

• struct kvm_cpuid_entry2 *entry;
• if (array->nent >= array->maxnent)

• return -E2BIG;
  

• struct kvm_cpuid_entry2 entry;

• entry = &array->entries[array->nent];
• entry->function = func;
• entry->index = 0;
• entry->flags = 0;

• memset(&entry, 0, sizeof(entry));
• entry.function = func;

switch (func) { case 0:

• entry->eax = 7;
  

• ++array->nent;
  

• entry.eax = 7;
  

  break; case 1:

• entry->ecx = F(MOVBE);
  

• ++array->nent;
  

• entry.ecx = F(MOVBE);
  

  break; case 7:

• entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
  

• entry->eax = 0;
  

• entry->ecx = F(RDPID);
  

• ++array->nent;
  

• default:

• entry.flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
  

• entry.eax = 0;
  

Nitpick: there's no need to set entry.eax = 0 as the whole structure was zeroed. Also, '|=' for flags could be just '='.

Agreed on dropping "entry.eax = 0". I could go either way on flags; I do like that the "|=" is consistent with do_host_cpuid().

...

• entry.ecx = F(RDPID);
  

  break;
• default:

• goto out;
  

  }

• if (array->nent >= array->maxnent)

• return -E2BIG;
  

• memcpy(&array->entries[array->nent++], &entry, sizeof(entry));

+out: return 0; }

Reviewed-by: Vitaly Kuznetsov vkuznets@redhat.com

-- Vitaly