Re: [PATCH v7 19/20] KVM: arm64: selftests: Add test case for Partitioned PMU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The KVM_ARM_VCPU_PMU_V3_ENABLE_PARTITION attribute expects an unsigned int, but a pointer to a 1-byte bool is passed, leading to an out-of-bounds stack read and potentially incorrect test execution.
- [Low] The 'pmu_partitioned' field added to 'struct vpmu_vm' is assigned but never read or utilized.
--

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

KVM: arm64: selftests: Add test case for Partitioned PMU

This commit adds testing for the Partitioned PMU by running all tests for
both Emulated and Partitioned PMU implementations. It modifies the setup
functions to take the implementation type as an argument and configures the
vCPU attribute accordingly.

> diff --git a/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c b/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c
> index ae36325c022fb..9702f1d43b832 100644
> --- a/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c
> +++ b/tools/testing/selftests/kvm/arm64/vpmu_counter_access.c
[ ... ]
>  struct vpmu_vm {
>  	struct kvm_vm *vm;
>  	struct kvm_vcpu *vcpu;
> +	bool pmu_partitioned;
>  };

This isn't a bug, but is the pmu_partitioned field used anywhere else in
this test? It looks like it is assigned during setup but never read later
in the code.

[ ... ]
> @@ -409,6 +420,13 @@ static void create_vpmu_vm(void *guest_code)
>  		.attr = KVM_ARM_VCPU_PMU_V3_IRQ,
>  		.addr = (uint64_t)&irq,
>  	};
> +	bool partition = (impl == PARTITIONED);
> +	struct kvm_device_attr part_attr = {
> +		.group = KVM_ARM_VCPU_PMU_V3_CTRL,
> +		.attr = KVM_ARM_VCPU_PMU_V3_ENABLE_PARTITION,
> +		.addr = (uint64_t)&partition
> +	};
> +	int ret;

Does this pass a 1-byte boolean to an attribute that expects a 4-byte
unsigned int?

When the kernel calls get_user() to read the
KVM_ARM_VCPU_PMU_V3_ENABLE_PARTITION attribute, could it read 3 bytes of
uninitialized stack data alongside the bool? 

If the uninitialized stack data contains non-zero bytes, could the kernel
evaluate the 32-bit value as true and erroneously enable PMU partitioning
even when partition is false?

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