linux-kernel - Re: [PATCH v2 3/3] KVM: SVM: WARN if GATag generation drops VM or vCPU ID information

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c653acea-d060-ed81-d3a0-c7379ecb0799@amd.com>
Date:   Thu, 16 Feb 2023 03:20:38 +0700
From:   "Suthikulpanit, Suravee" <suravee.suthikulpanit@....com>
To:     Sean Christopherson <seanjc@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Alejandro Jimenez <alejandro.j.jimenez@...cle.com>
Subject: Re: [PATCH v2 3/3] KVM: SVM: WARN if GATag generation drops VM or
 vCPU ID information



On 2/7/2023 7:21 AM, Sean Christopherson wrote:
> WARN if generating a GATag given a VM ID and vCPU ID doesn't yield the
> same IDs when pulling the IDs back out of the tag.  Don't bother adding
> error handling to callers, this is very much a paranoid sanity check as
> KVM fully controls the VM ID and is supposed to reject too-big vCPU IDs.
> 
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
>   arch/x86/kvm/svm/avic.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index 326341a22153..cfc8ab773025 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -43,12 +43,21 @@
>   #define AVIC_VM_ID_SHIFT		HWEIGHT32(AVIC_PHYSICAL_MAX_INDEX_MASK)
>   #define AVIC_VM_ID_MASK			(GENMASK(31, AVIC_VM_ID_SHIFT) >> AVIC_VM_ID_SHIFT)
>   
> -#define AVIC_GATAG(x, y)		(((x & AVIC_VM_ID_MASK) << AVIC_VM_ID_SHIFT) | \
> -						(y & AVIC_VCPU_ID_MASK))
>   #define AVIC_GATAG_TO_VMID(x)		((x >> AVIC_VM_ID_SHIFT) & AVIC_VM_ID_MASK)
>   #define AVIC_GATAG_TO_VCPUID(x)		(x & AVIC_VCPU_ID_MASK)
>   
> -static_assert(AVIC_GATAG(AVIC_VM_ID_MASK, AVIC_VCPU_ID_MASK) == -1u);
> +#define __AVIC_GATAG(vm_id, vcpu_id)	((((vm_id) & AVIC_VM_ID_MASK) << AVIC_VM_ID_SHIFT) | \
> +					 ((vcpu_id) & AVIC_VCPU_ID_MASK))
> +#define AVIC_GATAG(vm_id, vcpu_id)					\
> +({									\
> +	u32 ga_tag = __AVIC_GATAG(vm_id, vcpu_id);			\
> +									\
> +	WARN_ON_ONCE(AVIC_GATAG_TO_VCPUID(ga_tag) != (vcpu_id));	\
> +	WARN_ON_ONCE(AVIC_GATAG_TO_VMID(ga_tag) != (vm_id));		\
> +	ga_tag;								\
> +})
> +
> +static_assert(__AVIC_GATAG(AVIC_VM_ID_MASK, AVIC_VCPU_ID_MASK) == -1u);
>   
>   static bool force_avic;
>   module_param_unsafe(force_avic, bool, 0444);

Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@....com>

Thanks,
Suravee