linux-kernel - Re: [RFC 4/5] selftests: KVM: SNP IOCTL test

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: <ZrY6Z4mbbohVRbEh@google.com>
Date: Fri, 9 Aug 2024 08:48:55 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: "Pratik R. Sampat" <pratikrajesh.sampat@....com>
Cc: kvm@...r.kernel.org, shuah@...nel.org, thomas.lendacky@....com, 
	michael.roth@....com, pbonzini@...hat.com, pgonda@...gle.com, 
	linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC 4/5] selftests: KVM: SNP IOCTL test

On Wed, Jul 10, 2024, Pratik R. Sampat wrote:
> Introduce testing of SNP ioctl calls. This patch includes both positive
> and negative tests of various parameters such as flags, page types and
> policies.
> 
> Signed-off-by: Pratik R. Sampat <pratikrajesh.sampat@....com>
> ---
>  .../selftests/kvm/x86_64/sev_smoke_test.c     | 119 +++++++++++++++++-
>  1 file changed, 118 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
> index 500c67b3793b..1d5c275c11b3 100644
> --- a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
> @@ -186,13 +186,130 @@ static void test_sev_launch(void *guest_code, uint32_t type, uint64_t policy)
>  	kvm_vm_free(vm);
>  }
>  
> +static int spawn_snp_launch_start(uint32_t type, uint64_t policy, uint8_t flags)
> +{
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	int ret;
> +
> +	vm = vm_sev_create_with_one_vcpu(type, NULL, &vcpu);

Is a vCPU actually necessary/interesting?

> +	ret = snp_vm_launch(vm, policy, flags);
> +	kvm_vm_free(vm);
> +
> +	return ret;
> +}
> +
> +static void test_snp_launch_start(uint32_t type, uint64_t policy)
> +{
> +	uint8_t i;
> +	int ret;
> +
> +	ret = spawn_snp_launch_start(type, policy, 0);

s/spawn/__test, because "spawn" implies there's something living after this.

> +	TEST_ASSERT(!ret,
> +		    "KVM_SEV_SNP_LAUNCH_START should not fail, invalid flag.");

This should go away once vm_sev_ioctl() handles the assertion, but this assert
message is bad (there's no invalid flag).

> +
> +	for (i = 1; i < 8; i++) {
> +		ret = spawn_snp_launch_start(type, policy, BIT(i));
> +		TEST_ASSERT(ret && errno == EINVAL,
> +			    "KVM_SEV_SNP_LAUNCH_START should fail, invalid flag.");

Print the flag, type, and policy.  In general, please think about what information
would be helpful if this fails.