Re: [PATCH v17 17/20] KVM: arm64: CCA: Provide an accurate register list

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] KVM_GET_REG_LIST fails to restrict core registers to the 9 GP registers for Realm VMs, exposing all core registers instead.
- [Medium] Realm VM system register restrictions are only applied to KVM_GET_REG_LIST, allowing userspace to bypass them via KVM_SET_ONE_REG and KVM_GET_ONE_REG.
--

commit 1a82d37b0064cc3f1fa34c6757432fa355d160a3
Author: Jean-Philippe Brucker <jpb@kernel.org>

KVM: arm64: CCA: Provide an accurate register list
    
Userspace can set a few registers with KVM_SET_ONE_REG (9 GP registers
at runtime, and 3 system registers during initialization). Update the
register list returned by KVM_GET_REG_LIST.

> Userspace can set a few registers with KVM_SET_ONE_REG (9 GP registers
> at runtime, and 3 system registers during initialization). Update the
> register list returned by KVM_GET_REG_LIST.

[Severity: Medium]
Does KVM_GET_REG_LIST successfully restrict core registers to just the 9 GP
registers for Realm VMs?

Looking at copy_core_reg_indices(), it loops over the entire kvm_regs array
without checking vcpu_is_rec(vcpu):

arch/arm64/kvm/guest.c:copy_core_reg_indices() {
	for (i = 0; i < sizeof(struct kvm_regs) / sizeof(__u32); i++) {
		u64 reg = KVM_REG_ARM64 | KVM_REG_ARM_CORE | i;
		int size = core_reg_size_from_offset(vcpu, i);

		if (size < 0)
			continue;
        ...
}

This appears to expose all core registers to the userspace array rather than
limiting it to the 9 GP registers mentioned in the commit message.

[ ... ]

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 44aae52c473d7..a58c1fc13ea17 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c

[ ... ]

> @@ -5693,11 +5693,26 @@ static bool copy_reg_to_user(const struct sys_reg_desc *reg, u64 __user **uind)
>  	return true;
>  }
>  
> +static inline bool kvm_realm_sys_reg_hidden_user(const struct kvm_vcpu *vcpu,
> +						 u64 reg)
> +{
> +	switch (reg) {
> +	case SYS_ID_AA64DFR0_EL1:
> +	case SYS_PMCR_EL0:
> +		return false;
> +	}
> +	return true;
> +}
> +
>  static int walk_one_sys_reg(const struct kvm_vcpu *vcpu,
>  			    const struct sys_reg_desc *rd,
>  			    u64 __user **uind,
>  			    unsigned int *total)
>  {
> +	if (vcpu_is_rec(vcpu) &&
> +	    kvm_realm_sys_reg_hidden_user(vcpu, reg_to_encoding(rd)))
> +		return 0;
> +

[Severity: Medium]
Are the Realm system register restrictions bypassed by KVM_SET_ONE_REG and
KVM_GET_ONE_REG?

While this patch correctly hides the registers from KVM_GET_REG_LIST by
checking kvm_realm_sys_reg_hidden_user() in walk_one_sys_reg(), it doesn't
appear to add a similar check to the ioctl handlers for setting or getting
individual registers.

For example, in kvm_sys_reg_set_user(), the code only checks sysreg_hidden():

arch/arm64/kvm/sys_regs.c:kvm_sys_reg_set_user() {
    ...
	r = id_to_sys_reg_desc(vcpu, id, table, num);
	if (!r || sysreg_hidden(vcpu, r))
		return -ENOENT;
    ...
}

The getter similarly fails to enforce the Realm system register restrictions.
Does this allow userspace to directly read or overwrite hidden system
registers on a Realm VM if they know the register ID, bypassing the CCA
register protection boundary?

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