linux-kernel - Re: [PATCH v2 05/49] KVM: selftests: Assert that the @cpuid passed to get_cpuid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6a8aee9425a47290c7401d4926041c0611d69ff6.camel@redhat.com>
Date: Thu, 04 Jul 2024 20:58:38 -0400
From: Maxim Levitsky <mlevitsk@...hat.com>
To: Sean Christopherson <seanjc@...gle.com>, Paolo Bonzini
 <pbonzini@...hat.com>,  Vitaly Kuznetsov <vkuznets@...hat.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org, Hou Wenlong
 <houwenlong.hwl@...group.com>, Kechen Lu <kechenl@...dia.com>, Oliver Upton
 <oliver.upton@...ux.dev>, Binbin Wu <binbin.wu@...ux.intel.com>, Yang
 Weijiang <weijiang.yang@...el.com>, Robert Hoo <robert.hoo.linux@...il.com>
Subject: Re: [PATCH v2 05/49] KVM: selftests: Assert that the @cpuid passed
 to get_cpuid_entry() is non-NULL

On Fri, 2024-05-17 at 10:38 -0700, Sean Christopherson wrote:
> Add a sanity check in get_cpuid_entry() to provide a friendlier error than
> a segfault when a test developer tries to use a vCPU CPUID helper on a
> barebones vCPU.
> 
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
>  tools/testing/selftests/kvm/lib/x86_64/processor.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> index c664e446136b..f0f3434d767e 100644
> --- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
> @@ -1141,6 +1141,8 @@ const struct kvm_cpuid_entry2 *get_cpuid_entry(const struct kvm_cpuid2 *cpuid,
>  {
>  	int i;
>  
> +	TEST_ASSERT(cpuid, "Must do vcpu_init_cpuid() first (or equivalent)");
> +
>  	for (i = 0; i < cpuid->nent; i++) {
>  		if (cpuid->entries[i].function == function &&
>  		    cpuid->entries[i].index == index)

Hi,

Maybe it is better to do this assert in __vcpu_get_cpuid_entry() because the assert might confuse the
reader, since it just tests for NULL but when it fails, it complains that you need to call some function.

There is also another call to get_cpuid_entry() in kvm_cpu_fms but this call can't have this issue.

Besides this nitpick, looks good to me.

Best regards,
	Maxim Levitsky