Re: [PATCH v3 22/27] KVM: s390: Add basic arm64 kvm module

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Calling `kvm_init_with_dev()` from a secondary/independent KVM architecture backend (`kvm-s390-arm64`) causes catastrophic double-initialization of the KVM core subsystem, leading to global state corruption and kernel panics.
- [High] Integer underflow in `kvm_max_guest_address()` causes KVM to report an invalid, massively incorrect maximum IPA size on host systems with limited memory.
--

commit cceba4a7a6d94b89a1513f32f00ede5f3c39fa4c
Author: Steffen Eiden <seiden@linux.ibm.com>

KVM: s390: Add basic arm64 kvm module

Add basic code for the new arm64 on s390 KVM implementation.
Add kernel module boilerplate code and trivial functions.

> diff --git a/arch/s390/kvm/arm64/arm.c b/arch/s390/kvm/arm64/arm.c
> new file mode 100644
> index 0000000000000..591e2e874891a
> --- /dev/null
> +++ b/arch/s390/kvm/arm64/arm.c
[ ... ]
> +static u64 kvm_max_guest_address(void)
> +{
> +	u64 max_addr;
> +
> +	if (sclp.hamax == U64_MAX)
> +		max_addr = TASK_SIZE_MAX;
> +	else
> +		max_addr = min_t(u64, TASK_SIZE_MAX, sclp.hamax);
> +	return ALIGN_DOWN(max_addr + 1, 1 << 30) - 1;

[Severity: High]
If the host system has less than 1GB of addressable memory (i.e.,
sclp.hamax < 1GB - 1), will ALIGN_DOWN() evaluate to 0 and cause the
subsequent subtraction of 1 to underflow to U64_MAX?

> +}
[ ... ]
> +u32 get_kvm_ipa_limit(void)
> +{
> +	return fls64(kvm_max_guest_address() + 1) - 1;

[Severity: High]
Following the underflow in kvm_max_guest_address(), would this result in
fls64(U64_MAX + 1) - 1?

Since U64_MAX + 1 is 0 and fls64(0) returns 0, this calculates to -1, which
would be cast to 0xFFFFFFFF. Does this inadvertently return an invalid 32-bit
limit of 0xFFFFFFFF to userspace capability checks?

> +}
[ ... ]
> +static int __init kvm_s390_arm64_init(void)
> +{
> +	if (!sclp.has_aef)
> +		return -ENXIO;
> +
> +	return kvm_init_with_dev(sizeof(struct kvm_vcpu), 0, THIS_MODULE,
> +				 KVM_DEV_NAME, MISC_DYNAMIC_MINOR);

[Severity: Critical]
Since kvm_init_with_dev() is a core KVM function that manipulates global
state, does calling it from a secondary KVM architecture backend cause a
double-initialization if the primary s390 KVM module is already active?

Loading this module while kvm.ko is active appears to invoke
INIT_LIST_HEAD() on a miscdevice already in the list, potentially corrupting
the doubly linked list and causing a panic on traversal.

Could it also allocate a duplicate kvm_vcpu_cache, overwriting the global
pointer and leaking the original cache?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529155050.2902245-1-seiden@linux.ibm.com?part=22