Re: [PATCH v2] RISC-V: KVM: Zicbo[m|z|p] block sizes should be always present in ONE_REG

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Silently ignoring writes to Zicbo block size registers when the extension is disabled allows userspace to bypass hardware block size validation, leading to guest memory corruption.
- [High] Unconditionally exposing Zicbo block sizes in KVM_GET_REG_LIST and validating them against current VCPU ISA state breaks VM migration from non-Zicbo hosts.
--

commit ae62f61922ea3db59d443a539b06f94dce05e2f4
Author: Anup Patel <anup.patel@oss.qualcomm.com>

RISC-V: KVM: Zicbo[m|z|p] block sizes should be always present in ONE_REG

This commit removes the host ISA extension checks for Zicbo[m|z|p] block size
registers in the ONE_REG interface. It changes the register set logic to
validate the block size only if the extension is currently enabled on the
VCPU, and unconditionally exposes these registers in KVM_GET_REG_LIST.

> diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
> index bb920e8923c93..a8333441192c5 100644
> --- a/arch/riscv/kvm/vcpu_onereg.c
> +++ b/arch/riscv/kvm/vcpu_onereg.c

[ ... ]

> @@ -144,21 +138,18 @@ static int kvm_riscv_vcpu_set_reg_config(struct kvm_vcpu *vcpu,
>  		}
>  		break;
>  	case KVM_REG_RISCV_CONFIG_REG(zicbom_block_size):
> -		if (kvm_riscv_isa_check_host(ZICBOM))
> -			return -ENOENT;
> -		if (reg_val != riscv_cbom_block_size)
> +		if (riscv_isa_extension_available(vcpu->arch.isa, ZICBOM) &&
> +		    reg_val != riscv_cbom_block_size)
>  			return -EINVAL;

[Severity: High]
Could this logic silently ignore writes and bypass validation?

If userspace temporarily disables the Zicbom extension on the VCPU and then
writes an unsupported block size to this register, the evaluation of:

    riscv_isa_extension_available(vcpu->arch.isa, ZICBOM)

will be false. KVM will then return 0 (success) instead of -EINVAL,
effectively ignoring the write.

If userspace later re-enables the extension, it might incorrectly believe
the unsupported block size was successfully configured and advertise it to
the guest. Since kvm_riscv_vcpu_get_reg_config() unconditionally returns
riscv_cbom_block_size, wouldn't this read/write asymmetry result in the
guest using an incorrect cache block size and potentially corrupting memory?

Additionally, could this temporal dependency on the VCPU ISA state break
VM migration?

When migrating from a host without Zicbo extensions (where block size is 0)
to a host with them, Zicbom might be enabled by default on the new VCPUs.
Userspace typically restores CONFIG registers before ISA_EXT registers.
If userspace attempts to restore zicbom_block_size as 0 while the extension
is enabled on the destination VCPU, this check will return -EINVAL and
abort the migration.

>  		break;
>  	case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
> -		if (kvm_riscv_isa_check_host(ZICBOZ))
> -			return -ENOENT;
> -		if (reg_val != riscv_cboz_block_size)
> +		if (riscv_isa_extension_available(vcpu->arch.isa, ZICBOZ) &&
> +		    reg_val != riscv_cboz_block_size)
>  			return -EINVAL;

[ ... ]

> @@ -614,20 +605,6 @@ static int copy_config_reg_indices(const struct kvm_vcpu *vcpu,
>  		u64 size;
>  		u64 reg;
>  
> -		/*
> -		 * Avoid reporting config reg if the corresponding extension
> -		 * was not available.
> -		 */
> -		if (i == KVM_REG_RISCV_CONFIG_REG(zicbom_block_size) &&
> -		    kvm_riscv_isa_check_host(ZICBOM))
> -			continue;

[Severity: High]
By removing this host ISA check, won't the register be unconditionally
exposed to KVM_GET_REG_LIST even on hosts that do not support the extension?

If a host lacks Zicbom, riscv_cbom_block_size will be 0. Exposing this 
register will cause userspace to serialize the value 0 during migration.
As noted above, attempting to restore this 0 value on a destination host
that supports Zicbom will then trigger an -EINVAL during restoration in
kvm_riscv_vcpu_set_reg_config().

> -		else if (i == KVM_REG_RISCV_CONFIG_REG(zicboz_block_size) &&
> -			 kvm_riscv_isa_check_host(ZICBOZ))
> -			continue;
> -		else if (i == KVM_REG_RISCV_CONFIG_REG(zicbop_block_size) &&
> -			 kvm_riscv_isa_check_host(ZICBOP))
> -			continue;
> -
>  		size = IS_ENABLED(CONFIG_32BIT) ? KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706083850.1425228-1-anup.patel@oss.qualcomm.com?part=1