linux-kernel - Re: [PATCH 2/2] KVM: xen: (re-)initialize shared

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:   Fri, 1 Dec 2023 15:32:40 +0000
From:   Paul Durrant <xadimgnik@...il.com>
To:     David Woodhouse <dwmw2@...radead.org>,
        Sean Christopherson <seanjc@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] KVM: xen: (re-)initialize shared_info if guest
 (32/64-bit) mode is set

On 01/12/2023 10:45, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@...zon.com>
> 
> If the shared_info PFN cache has already been initialized then the content
> of the shared_info page needs to be (re-)initialized if the guest mode is
> set. It is no lnger done when the PFN cache is activated.
> Setting the guest mode is either done explicitly by the VMM via the
> KVM_XEN_ATTR_TYPE_LONG_MODE attribute, or implicitly when the guest writes
> the MSR to set up the hypercall page.
> 
> Signed-off-by: Paul Durrant <pdurrant@...zon.com>
> ---
>   arch/x86/kvm/xen.c | 20 +++++++++++++++-----
>   1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 7bead3f65e55..bfc8f6698cbc 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -624,8 +624,15 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
>   		} else {
>   			mutex_lock(&kvm->arch.xen.xen_lock);
>   			kvm->arch.xen.long_mode = !!data->u.long_mode;
> +
> +			/*
> +			 * If shared_info has already been initialized
> +			 * then re-initialize it with the new width.
> +			 */
> +			r = kvm->arch.xen.shinfo_cache.active ?
> +				kvm_xen_shared_info_init(kvm) : 0;
> +
>   			mutex_unlock(&kvm->arch.xen.xen_lock);
> -			r = 0;
>   		}
>   		break;
>   
> @@ -657,9 +664,6 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
>   		}
>   		srcu_read_unlock(&kvm->srcu, idx);
>   
> -		if (!r && kvm->arch.xen.shinfo_cache.active)
> -			r = kvm_xen_shared_info_init(kvm);
> -
>   		mutex_unlock(&kvm->arch.xen.xen_lock);
>   		break;
>   	}
> @@ -1144,7 +1148,13 @@ int kvm_xen_write_hypercall_page(struct kvm_vcpu *vcpu, u64 data)
>   	bool lm = is_long_mode(vcpu);
>   
>   	/* Latch long_mode for shared_info pages etc. */
> -	vcpu->kvm->arch.xen.long_mode = lm;
> +	kvm->arch.xen.long_mode = lm;
> +
> +	if (kvm->arch.xen.shinfo_cache.active &&
> +	    kvm_xen_shared_info_init(kvm)) {
> +		mutex_unlock(&kvm->arch.xen.xen_lock);

This unlock is bogus; it should have been removed. I'll send a v2.

   Paul

> +		return 1;
> +	}
>   
>   	/*
>   	 * If Xen hypercall intercept is enabled, fill the hypercall