Re: [PATCH 18/21] KVM: arm64: Add vCPU device attr to partition the PMU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unprivileged host userspace can trigger a host kernel panic (NULL pointer dereference) by calling KVM_ARM_VCPU_PMU_V3_ENABLE_PARTITION before the VM's PMU is initialized.
- [Medium] `kvm_arm_pmu_v3_has_attr()` incorrectly advertises support for the `KVM_ARM_VCPU_PMU_V3_ENABLE_PARTITION` attribute on systems that do not actually support it, breaking live migration.
--

commit 69324cfdaa77bd05919e5ffe00139f0a4c7a3e0c
Author: Colton Lewis <coltonlewis@google.com>

KVM: arm64: Add vCPU device attr to partition the PMU

Add a new PMU device attr to enable the partitioned PMU for a given
VM. This capability can be set when the PMU is initially configured
before the vCPU starts running.

> diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> index f5ee18b4dfae7..4e15948ac2565 100644
> --- a/arch/arm64/kvm/pmu.c
> +++ b/arch/arm64/kvm/pmu.c
> @@ -769,6 +769,28 @@ int kvm_arm_pmu_v3_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
>  
>  		return kvm_arm_pmu_v3_set_nr_counters(vcpu, n);
>  	}
> +	case KVM_ARM_VCPU_PMU_V3_ENABLE_PARTITION: {
> +		unsigned int __user *uaddr = (unsigned int __user *)(long)attr->addr;
> +		u32 val;
> +
> +		if (get_user(val, uaddr))
> +			return -EFAULT;
> +
> +		if (!has_kvm_pmu_partition_support())
> +			return -EPERM;
> +
> +		if (kvm_vm_has_ran_once(kvm))
> +			return -EBUSY;
> +
> +		kvm_pmu_partition_enable(kvm, val);
> +		if (val) {
> +			unsigned int max_counters = kvm_arm_pmu_get_max_counters(kvm);

[Severity: High]
Can an unprivileged host userspace process trigger a NULL pointer
dereference here if the PMU is not yet initialized?

If KVM_ARM_VCPU_PMU_V3_ENABLE_PARTITION is called before the PMU is
initialized, kvm->arch.arm_pmu will be NULL.

When this code calls kvm_arm_pmu_get_max_counters(), it dereferences the
NULL pointer inside bitmap_weight():

arch/arm64/kvm/pmu.c:kvm_arm_pmu_get_max_counters() {
    struct arm_pmu *arm_pmu = kvm->arch.arm_pmu;
    ...
    return bitmap_weight(arm_pmu->cntr_mask, ARMV8_PMU_MAX_GENERAL_COUNTERS);
}

> +
> +			if (kvm->arch.nr_pmu_counters > max_counters)
> +				kvm_arm_set_nr_counters(kvm, max_counters);
> +		}
> +		return 0;
> +	}
[ ... ]
> @@ -808,6 +830,7 @@ int kvm_arm_pmu_v3_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
>  	case KVM_ARM_VCPU_PMU_V3_FILTER:
>  	case KVM_ARM_VCPU_PMU_V3_SET_PMU:
>  	case KVM_ARM_VCPU_PMU_V3_SET_NR_COUNTERS:
> +	case KVM_ARM_VCPU_PMU_V3_ENABLE_PARTITION:
>  		if (kvm_vcpu_has_pmu(vcpu))
>  			return 0;
>  	}

[Severity: Medium]
Does this incorrectly advertise support for KVM_ARM_VCPU_PMU_V3_ENABLE_PARTITION
on systems that don't support it?

If kvm_arm_pmu_v3_has_attr() returns 0 as long as the VCPU has a PMU, VMMs
might assume the partition capability is available.

However, setting this attribute in kvm_arm_pmu_v3_set_attr() explicitly
rejects the configuration if has_kvm_pmu_partition_support() is false:

arch/arm64/kvm/pmu.c:kvm_arm_pmu_v3_set_attr() {
    ...
    case KVM_ARM_VCPU_PMU_V3_ENABLE_PARTITION: {
        ...
        if (!has_kvm_pmu_partition_support())
            return -EPERM;
    }
}

Could this inconsistency cause live migrations to fail when the VMM attempts
to restore this attribute on a destination that lacks partition support?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612192909.1153907-1-coltonlewis@google.com?part=18