linux-kernel - Re: [PATCH 02/20] KVM: selftests: Sync dirty_log_test iteration to guest *before* resuming

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: <f91364794fd3d08e2eb142b53a4e18c5b05cb7c4.camel@redhat.com>
Date: Tue, 17 Dec 2024 18:58:53 -0500
From: Maxim Levitsky <mlevitsk@...hat.com>
To: Sean Christopherson <seanjc@...gle.com>, Paolo Bonzini
 <pbonzini@...hat.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org, Peter Xu
	 <peterx@...hat.com>
Subject: Re: [PATCH 02/20] KVM: selftests: Sync dirty_log_test iteration to
 guest *before* resuming

On Fri, 2024-12-13 at 17:07 -0800, Sean Christopherson wrote:
> Sync the new iteration to the guest prior to restarting the vCPU, otherwise
> it's possible for the vCPU to dirty memory for the next iteration using the
> current iteration's value.
> 
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
>  tools/testing/selftests/kvm/dirty_log_test.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> index cdae103314fc..41c158cf5444 100644
> --- a/tools/testing/selftests/kvm/dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> @@ -859,9 +859,9 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  		 */
>  		if (++iteration == p->iterations)
>  			WRITE_ONCE(host_quit, true);
> -
> -		sem_post(&sem_vcpu_cont);
>  		sync_global_to_guest(vm, iteration);
> +
> +		sem_post(&sem_vcpu_cont);
>  	}
>  
>  	pthread_join(vcpu_thread, NULL);


AFAIK, this patch doesn't 100% gurantee that this won't happen:

The READ_ONCE that guest uses only guarntees no wierd compiler optimizations are used.
The guest can still read the iteration value to a register, get #vmexit, after which the iteration
will be increased and then write the old value.


Is this worth to reorder this to decrease the chances of this happening? I am not sure, as this will
just make this problem rarer and thus harder to debug.
Currently the test just assumes that this can happen and deals with this.


Best regards,
      Maxim Levitsky