linux-kernel - Re: KVM: Warn if mark_page_dirty() is called without an active vCPU

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:   Mon, 22 Nov 2021 17:01:59 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     David Woodhouse <dwmw2@...radead.org>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        butt3rflyh4ck <butterflyhuangxx@...il.com>, kvm@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: KVM: Warn if mark_page_dirty() is called without an active vCPU

On Sat, Nov 20, 2021, David Woodhouse wrote:
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 6c5083f2eb50..72c6453bcef4 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3020,12 +3020,17 @@ void mark_page_dirty_in_slot(struct kvm *kvm,
>  			     struct kvm_memory_slot *memslot,
>  		 	     gfn_t gfn)
>  {
> +	struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
> +
> +	if (WARN_ON_ONCE(!vcpu) || WARN_ON_ONCE(vcpu->kvm != kvm))

Maybe use KVM_BUG_ON?  And two separate WARNs are probably overkill.

	if (KVM_BUG_ON(!vcpu || vcpu->kvm != kvm, kvm))


I'd also prefer to not retrieve the vCPU in the dirty_bitmap path, at least not
until it's necessary (for the proposed dirty quota throttling), though that's not
a strong preference.

> +		return;
> +
>  	if (memslot && kvm_slot_dirty_track_enabled(memslot)) {
>  		unsigned long rel_gfn = gfn - memslot->base_gfn;
>  		u32 slot = (memslot->as_id << 16) | memslot->id;
>  
>  		if (kvm->dirty_ring_size)
> -			kvm_dirty_ring_push(kvm_dirty_ring_get(kvm),
> +			kvm_dirty_ring_push(&vcpu->dirty_ring,
>  					    slot, rel_gfn);

This can now squeeze on a single line.

			kvm_dirty_ring_push(&vcpu->dirty_ring, slot, rel_gfn);

>  		else
>  			set_bit_le(rel_gfn, memslot->dirty_bitmap);
> -- 
> 2.31.1
>