linux-kernel - Re: [PATCH] selftests/kvm: add test for KVM_GET_MSR_FEATURE_INDEX

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]

Date:   Wed, 17 Mar 2021 11:49:43 +0100
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Emanuele Giuseppe Esposito <eesposit@...hat.com>,
        linux-kselftest@...r.kernel.org
Cc:     Shuah Khan <shuah@...nel.org>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Andrew Jones <drjones@...hat.com>,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH] selftests/kvm: add test for
 KVM_GET_MSR_FEATURE_INDEX_LIST

On 17/03/21 08:45, Emanuele Giuseppe Esposito wrote:
> +	struct kvm_msr_list features_list;
>   
>   	buffer.header.nmsrs = 1;
>   	buffer.entry.index = msr_index;
> +	features_list.nmsrs = 1;
> +
>   	kvm_fd = open(KVM_DEV_PATH, O_RDONLY);
>   	if (kvm_fd < 0)
>   		exit(KSFT_SKIP);
>   
> +	r = ioctl(kvm_fd, KVM_GET_MSR_FEATURE_INDEX_LIST, &features_list);
> +	TEST_ASSERT(r < 0 && r != -E2BIG, "KVM_GET_MSR_FEATURE_INDEX_LIST IOCTL failed,\n"
> +		"  rc: %i errno: %i", r, errno);

Careful: because this has nsmrs == 1, you are overwriting an u32 of the 
stack after struct kvm_msr_list.  You need to use your own struct 
similar to what is done with "buffer.header" and "buffer.entry".

>   	r = ioctl(kvm_fd, KVM_GET_MSRS, &buffer.header);
>   	TEST_ASSERT(r == 1, "KVM_GET_MSRS IOCTL failed,\n"
>   		"  rc: %i errno: %i", r, errno);
> 

More in general, this is not a test, but rather a library function used 
to read a single MSR.

If you would like to add a test for KVM_GET_MSR_FEATURE_INDEX_LIST that 
would be very welcome.  That would be a new executable.  Looking at the 
logic for the ioctl, the main purpose of the test should be:

- check that if features_list.nmsrs is too small it will set the nmsrs 
field and return -E2BIG.

- check that all MSRs returned by KVM_GET_MSR_FEATURE_INDEX_LIST can be 
accessed with KVM_GET_MSRS

So something like this:

   set nmsrs to 0 and try the ioctl
   check that it returns -E2BIG and has changed nmsrs
   if nmsrs != 1 {
     set nmsrs to 1 and try the ioctl again
     check that it returns -E2BIG
   }
   malloc a buffer with room for struct kvm_msr_list and nmsrs indices
   set nmsrs in the malloc-ed buffer and try the ioctl again
   for each index
     invoke kvm_get_feature_msr to read it

(The test should also be skipped if KVM does not expose the 
KVM_CAP_GET_MSR_FEATURES capability).

Thanks,

Paolo