Re: [PATCH 02/22] KVM: arm64: nv: Consolidate computation of stage-2 permissions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but this patch silently fixes a critical pre-existing security vulnerability in nested KVM where stage-2 Execute-Never (XN) restrictions were ignored when the host lacks FEAT_XNX. The bug should be explicitly noted and split into a separate fix for stable backporting.
--

commit 60a1129130ae1d928ae1a5f97eb06ae059dacb1a
Author: Oliver Upton <oupton@kernel.org>
Subject: KVM: arm64: nv: Consolidate computation of stage-2 permissions

This commit centralizes the computation of stage-2 permissions within the
table walk rather than inspecting the descriptor directly for execution
permissions.

> diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h
> --- a/arch/arm64/include/asm/kvm_nested.h
> +++ b/arch/arm64/include/asm/kvm_nested.h
> @@ -129,34 +131,12 @@ static inline bool kvm_has_xnx(struct kvm *kvm)
>  
>  static inline bool kvm_s2_trans_exec_el0(struct kvm *kvm, struct kvm_s2_trans *trans)
>  {
> -	u8 xn = FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, trans->desc);
> -
> -	if (!kvm_has_xnx(kvm))
> -		xn &= FIELD_PREP(KVM_PTE_LEAF_ATTR_HI_S2_XN, 0b10);

[ ... ]

> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -241,6 +241,36 @@ static int swap_guest_s2_desc(struct kvm_vcpu *vcpu, phys_addr_t pa, u64 old, u6
>  	return __kvm_at_swap_desc(vcpu->kvm, pa, old, new);
>  }
>  
> +static void compute_s2_permissions(struct kvm_vcpu *vcpu, struct s2_walk_info *wi,
> +				   struct s2_walk_step *ws, struct kvm_s2_trans *trans)
> +{
> +	u8 s2ap = FIELD_GET(KVM_PTE_LEAF_ATTR_LO_S2_S2AP, ws->desc);
> +	u8 xn = FIELD_GET(KVM_PTE_LEAF_ATTR_HI_S2_XN, ws->desc);
> +
> +	if (!kvm_has_xnx(vcpu->kvm))
> +		xn &= 0b10;

[Severity: Critical]
This is a pre-existing issue, but this change silently fixes a regression
in nested virtualization isolation.

In the old code for kvm_s2_trans_exec_el0 and kvm_s2_trans_exec_el1, the
masking was done using FIELD_PREP. Because xn was a u8 and FIELD_PREP 
returned a 64-bit value shifted by 53 bits, the bitwise AND always 
evaluated to 0. 

As a result, the check bypassed the L1 hypervisor's Execute-Never
restrictions entirely when the host lacked the hardware feature.

Since xn &= 0b10 correctly resolves this in the new compute_s2_permissions
function, does it make sense to split this specific correction into a
separate commit so it can be easily backported to stable trees?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623184201.1518871-1-oupton@kernel.org?part=2