Re: [PATCH v4 4/7] KVM: VMX: Implement LASS violation check

["Chang S. Bae" <chang.seok.bae@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2666 bytes --]

On 8/5/2026 6:15 PM, Sohil Mehta wrote:
> 
> +bool vmx_is_lass_violation(struct kvm_vcpu *vcpu, gva_t gva,
> +			   unsigned int size, unsigned int flags)
> +{
> +	const bool is_supervisor_address = !!(gva & BIT_ULL(63));
> +	const bool implicit_supervisor = !!(flags & X86EMUL_F_IMPLICIT);
> +	const bool fetch = !!(flags & X86EMUL_F_FETCH);
> +
> +	if (!kvm_is_cr4_bit_set(vcpu, X86_CR4_LASS) || !is_long_mode(vcpu))
> +		return false;
> +
> +	/*
> +	 * INVLPG isn't subject to LASS, e.g. to allow invalidating userspace
> +	 * addresses without toggling RFLAGS.AC.  Branch targets aren't subject
> +	 * to LASS in order to simplify far control transfers (the subsequent
> +	 * fetch will enforce LASS as appropriate).
> +	 */
> +	if (flags & (X86EMUL_F_BRANCH | X86EMUL_F_INVLPG))
> +		return false;
> +
> +	if (!implicit_supervisor && vmx_get_cpl(vcpu) == 3)
> +		return is_supervisor_address;
> +
> +	/*
> +	 * LASS enforcement for supervisor-mode data accesses depends on SMAP
> +	 * being enabled, and like SMAP ignores explicit accesses if RFLAGS.AC=1.
> +	 */
> +	if (!fetch) {
> +		if (!kvm_is_cr4_bit_set(vcpu, X86_CR4_SMAP))
> +			return false;
> +
> +		if (!implicit_supervisor && (kvm_get_rflags(vcpu) & X86_EFLAGS_AC))
> +			return false;
> +	}
> +
> +	/*
> +	 * The entire access must be in the appropriate address space.  Note,
> +	 * if LAM is supported, @gva has already been untagged, so barring a
> +	 * massive architecture change to expand the canonical address range,
> +	 * it's impossible for a user access to straddle user and supervisor
> +	 * address spaces.
> +	 */
> +	if (size && !((gva + size - 1) & BIT_ULL(63)))
> +		return true;
> +
> +	return !is_supervisor_address;
> +}

This looks like an essential piece for the check introduced in the 
series. I could follow the logic, and it looks sane to me. If allowed,

   Reviewed-by: Chang S. Bae <chang.seok.bae@intel.com>

The code also looks to be written according to Sean's comment in:

   https://lore.kernel.org/kvm/ZNwOYdy3AC12MI52@google.com

Yes, this could be a matter of taste as taking and going to maintain the 
code. But as a reader looking at this, the logic was not that easy to 
capture at a high level.

So what the the bottom half code does seems to be something like:

   if in user mode,
       violated if gva is a supervisor address

   if in supervisor mode,
       if LASS enforcement is applicable,
           violated if either gva or its access end is an user address

Maybe the high-level logic could be highlighted first. Attached a diff 
to help figuring out that option just in case. Feel free to ignore if it 
doesn't matter that much.

Thanks,
Chang

[-- Attachment #2: tmp.diff --]
[-- Type: text/x-patch, Size: 5883 bytes --]

diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index 4c52ab8d0786..5e2a703e4be6 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -1031,6 +1031,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
 	.vcpu_deliver_sipi_vector = kvm_vcpu_deliver_sipi_vector,
 
 	.get_untagged_addr = vmx_get_untagged_addr,
+	.is_lass_violation = vmx_is_lass_violation,
 
 	.mem_enc_ioctl = vt_op_tdx_only(mem_enc_ioctl),
 	.vcpu_mem_enc_ioctl = vt_op_tdx_only(vcpu_mem_enc_ioctl),
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 151873407abd..3ad5374d45fc 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -5308,11 +5308,12 @@ int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification,
 			*ret = off;
 
 		*ret = vmx_get_untagged_addr(vcpu, *ret, 0);
-		/* Long mode: #GP(0)/#SS(0) if the memory address is in a
-		 * non-canonical form. This is the only check on the memory
-		 * destination for long mode!
+		/*
+		 * Long mode: #GP(0)/#SS(0) if the memory address is in a
+		 * non-canonical form, or if the access violates LASS.
 		 */
-		exn = is_noncanonical_address(*ret, vcpu, 0);
+		exn = is_noncanonical_address(*ret, vcpu, 0) ||
+		      vmx_is_lass_violation(vcpu, *ret, len, 0);
 	} else {
 		/*
 		 * When not in long mode, the virtual/linear address is
@@ -6117,7 +6118,7 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
 	if (type != VMX_VPID_EXTENT_ALL_CONTEXT && !operand.vpid)
 		return nested_vmx_fail(vcpu, VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
 
-	/* LAM doesn't apply to addresses that are inputs to TLB invalidation. */
+	/* LAM and LASS don't apply to addresses that are inputs to TLB invalidation. */
 	if (type == VMX_VPID_EXTENT_INDIVIDUAL_ADDR &&
 	    is_noncanonical_invlpg_address(operand.gla, vcpu))
 		return nested_vmx_fail(vcpu, VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
diff --git a/arch/x86/kvm/vmx/sgx.c b/arch/x86/kvm/vmx/sgx.c
index 771c75a58343..4ac305ed6dea 100644
--- a/arch/x86/kvm/vmx/sgx.c
+++ b/arch/x86/kvm/vmx/sgx.c
@@ -39,7 +39,8 @@ static int sgx_get_encls_gva(struct kvm_vcpu *vcpu, unsigned long offset,
 		fault = true;
 	} else if (likely(is_64_bit_mode(vcpu))) {
 		*gva = vmx_get_untagged_addr(vcpu, *gva, 0);
-		fault = is_noncanonical_address(*gva, vcpu, 0);
+		fault = is_noncanonical_address(*gva, vcpu, 0) ||
+			vmx_is_lass_violation(vcpu, *gva, size, 0);
 	} else {
 		*gva &= 0xffffffff;
 		fault = (s.unusable) ||
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index b6634637f1ca..0a49dc92eb49 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -8604,6 +8604,69 @@ gva_t vmx_get_untagged_addr(struct kvm_vcpu *vcpu, gva_t gva, unsigned int flags
 	return (sign_extend64(gva, lam_bit) & ~BIT_ULL(63)) | (gva & BIT_ULL(63));
 }
 
+static bool is_user_mode(struct kvm_vcpu *vcpu, unsigned int flags)
+{
+	return !(flags & X86EMUL_F_IMPLICIT) && vmx_get_cpl(vcpu) == 3;
+}
+
+static bool is_supervisor_address(gva_t gva)
+{
+	return gva & BIT_ULL(63);
+}
+
+/*
+ * The entire access must be in the appropriate address space.  Note, if LAM is
+ * supported, @gva has already been untagged, so barring a massive architecture
+ * change to expand the canonical address range, it's impossible for a user
+ * access to straddle user and supervisor address spaces.
+ */
+static bool is_user_address(gva_t gva, unsigned int size)
+{
+	return !is_supervisor_address(gva) || (size && !is_supervisor_address(gva + size - 1));
+}
+
+/*
+ * LASS enforcement for supervisor-mode data accesses depends on being enabled,
+ * and like SMAP ignores explicit accesses if RFLAGS.AC=1.
+ */
+static bool is_lass_enforced(struct kvm_vcpu *vcpu, unsigned int flags)
+{
+	if (flags & X86EMUL_F_FETCH)
+		return true;
+
+	if (!kvm_is_cr4_bit_set(vcpu, X86_CR4_SMAP))
+		return false;
+
+	if (!(flags & X86EMUL_F_IMPLICIT) && (kvm_get_rflags(vcpu) & X86_EFLAGS_AC))
+		return false;
+
+	return true;
+}
+
+bool vmx_is_lass_violation(struct kvm_vcpu *vcpu, gva_t gva,
+			   unsigned int size, unsigned int flags)
+{
+	if (!kvm_is_cr4_bit_set(vcpu, X86_CR4_LASS) || !is_long_mode(vcpu))
+		return false;
+
+	/*
+	 * INVLPG isn't subject to LASS, e.g. to allow invalidating userspace
+	 * addresses without toggling RFLAGS.AC.  Branch targets aren't subject
+	 * to LASS in order to simplify far control transfers (the subsequent
+	 * fetch will enforce LASS as appropriate).
+	 */
+	if (flags & (X86EMUL_F_BRANCH | X86EMUL_F_INVLPG))
+		return false;
+
+	if (is_user_mode(vcpu, flags)) {
+		return is_supervisor_address(gva);
+	} else {
+		if (!is_lass_enforced(vcpu, flags))
+			return false;
+		return is_user_address(gva, size);
+	}
+}
+
 static unsigned int vmx_handle_intel_pt_intr(void)
 {
 	struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index dc8517f15bc4..43df725a77f0 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -397,6 +397,9 @@ u64 vmx_get_l2_tsc_multiplier(struct kvm_vcpu *vcpu);
 
 gva_t vmx_get_untagged_addr(struct kvm_vcpu *vcpu, gva_t gva, unsigned int flags);
 
+bool vmx_is_lass_violation(struct kvm_vcpu *vcpu, gva_t gva,
+			   unsigned int size, unsigned int flags);
+
 void vmx_update_cpu_dirty_logging(struct kvm_vcpu *vcpu);
 
 u64 vmx_get_supported_debugctl(struct kvm_vcpu *vcpu, bool host_initiated);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7662f6901a95..cd21dda27c03 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10758,7 +10758,7 @@ int kvm_handle_invpcid(struct kvm_vcpu *vcpu, unsigned long type, gva_t gva)
 	switch (type) {
 	case INVPCID_TYPE_INDIV_ADDR:
 		/*
-		 * LAM doesn't apply to addresses that are inputs to TLB
+		 * LAM and LASS don't apply to addresses that are inputs to TLB
 		 * invalidation.
 		 */
 		if ((!pcid_enabled && (operand.pcid != 0)) ||