Re: [PATCH v18 01/23] KVM: arm64: protected VM: Handle set_one_reg CNTVCT_EL0/CNTPCT_EL0

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: arm64: pKVM: protected VM timer offset vulnerable to host TOCTOU
--

commit 960552f9d7645214acac73b01e88ba381530e333
Author: Suzuki K Poulose <suzuki.poulose@arm.com>
Date:   Tue Sep 15 16:01:19 2026 +0000

    KVM: arm64: protected VM: Handle set_one_reg CNTVCT_EL0/CNTPCT_EL0
    
This commit fixes a WARN() in timer_set_offset() caused by userspace writing
to CNTVCT/CNTPCT registers for protected VMs. It accomplishes this by setting
the timer offsets to 0 and marking the offset initialized at pKVM init time,
removing the need for the vm_offset pointer to be NULL.

Link: https://lore.kernel.org/all/20260908164641.416911F00A3A@smtp.kernel.org

[ ... ]
> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> index 459bd9eb7e4bc..e7b38eff63bd1 100644
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> @@ -528,19 +528,19 @@ static int init_pkvm_hyp_vcpu(struct pkvm_hyp_vcpu *hyp_vcpu,
>  	hyp_vcpu->vcpu.arch.cflags = READ_ONCE(host_vcpu->arch.cflags);
>  	hyp_vcpu->vcpu.arch.mp_state.mp_state = KVM_MP_STATE_STOPPED;
>  
> -	if (!pkvm_hyp_vcpu_is_protected(hyp_vcpu)) {
> -		/*
> -		 * Timer offsets are pointing to the untrusted KVM copy,
> -		 * which is pinned in __pkvm_init_vm() for the VM life time.
> -		 * It is worth noting that hyp_vm->host_kvm points to an EL2
> -		 * linear map address and timer_get_offset() will use
> -		 * kern_hyp_va() which is safe as it is idempotent.
> -		 */
> -		vcpu_vtimer(&hyp_vcpu->vcpu)->offset.vm_offset =
> -			&hyp_vm->host_kvm->arch.timer_data.voffset;
> -		vcpu_ptimer(&hyp_vcpu->vcpu)->offset.vm_offset =
> -			&hyp_vm->host_kvm->arch.timer_data.poffset;
> -	}
> +	/*
> +	 * Timer offsets are pointing to the untrusted KVM copy,
> +	 * which is pinned in __pkvm_init_vm() for the VM life time.
> +	 * It is worth noting that hyp_vm->host_kvm points to an EL2
> +	 * linear map address and timer_get_offset() will use
> +	 * kern_hyp_va() which is safe as it is idempotent.
> +	 * Also for protected VMs the offset is fixed to 0 and is prevented
> +	 * from changing.
> +	 */
> +	vcpu_vtimer(&hyp_vcpu->vcpu)->offset.vm_offset =
> +		&hyp_vm->host_kvm->arch.timer_data.voffset;
> +	vcpu_ptimer(&hyp_vcpu->vcpu)->offset.vm_offset =
> +		&hyp_vm->host_kvm->arch.timer_data.poffset;

[Severity: High]
Does this change introduce a TOCTOU regression for protected VMs? By removing
the !pkvm_hyp_vcpu_is_protected() check in init_pkvm_hyp_vcpu(), the protected
VM's vm_offset is now unconditionally assigned to point to the untrusted host
memory back-pointer (hyp_vm->host_kvm).

Under the pKVM threat model, the host kernel is untrusted. If the host
modifies the voffset or poffset in its timer_data at runtime, EL2 will read
this manipulated value. This could allow a compromised host kernel to alter
the virtual and physical timer offsets observed by a protected VM, violating
guest integrity.

Should the offset instead be derived from the EL2-private &hyp_vm->kvm
structure or statically set to 0 for protected VMs?

[ ... ]
> diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
> index bc6f2fdd7ad33..4f0aa3bb69f45 100644
> --- a/include/kvm/arm_arch_timer.h
> +++ b/include/kvm/arm_arch_timer.h
> @@ -176,8 +176,7 @@ static inline bool has_cntpoff(void)
>  		if (__ctxt) {						\
>  			struct arch_timer_offset *ato = &__ctxt->offset;\
>  									\
> -			if (ato->vm_offset)				\
> -				off += *KERN_HYP_VA(ato->vm_offset);	\
> +			off += *KERN_HYP_VA(ato->vm_offset);		\
>  			if (ato->vcpu_offset)				\
>  				off += *KERN_HYP_VA(ato->vcpu_offset);	\
>  		}							\

[Severity: High]
Since the pointer is now unconditionally assigned to host memory for protected
VMs, EL2 will dereference this host-controlled pointer here in
timer_get_offset() via KERN_HYP_VA(). Can this cause the hypervisor to read a
security-sensitive value from untrusted host memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915160141.3543048-1-suzuki.poulose@arm.com?part=1