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

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 08/09/2026 18:00, sashiko-bot@kernel.org wrote:
> 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.

That is correct, but a later patch actually restricts this for set_reg()
calls. I will pull that into this series and will also fixup the commit
description, potentially folding that one here.

> 
> [ ... ]
> 
>> 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():

As above, this is in a separate patch, I will fix this in next version.


Suzuki