Re: [PATCH v6 29/33] KVM: s390: arm64: Implement required functions

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Uninitialized lock acquisition causes lockdep splat or kernel crash.
- [High] Empty exception injection stubs cause infinite VCPU exit loops.
--

--- Patch [29]: [PATCH v6 29/33] KVM: s390: arm64: Implement required functions ---
commit 7893f22f5b221e3beeba9913544dc8c7efcfbfc0
Author: Steffen Eiden <[email protected]>

KVM: s390: arm64: Implement required functions
    
Implement the mostly trivial functions that the shared arm64 (kvm)
code & headers oblige s390 to implement.

Implement a very basic smccc handler that (non-compliantly) is just able
to stop a vcpu.

> diff --git a/arch/s390/kvm/arm64/handle_exit.c b/arch/s390/kvm/arm64/handle_exit.c
> new file mode 100644
> --- /dev/null
> +++ b/arch/s390/kvm/arm64/handle_exit.c
[ ... ]
> +static int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
> +{
> +	u32 func_id = vcpu_get_reg(vcpu, 0);
> +	u64 val = PSCI_RET_NOT_SUPPORTED;
> +	int ret = 1;
> +
> +	if (func_id == PSCI_0_2_FN_SYSTEM_OFF) {
> +		scoped_guard(spinlock, &vcpu->arch.mp_state_lock) {

[Severity: High]
Can this cause a lockdep splat or kernel crash?

This lock is an arm64-specific state that does not appear to be initialized 
via spin_lock_init() in the s390 VCPU creation path. 

If a guest triggers an SMC/HVC trap with func_id == PSCI_0_2_FN_SYSTEM_OFF, 
the host handles this in kvm_smccc_call_handler() and attempts to acquire 
vcpu->arch.mp_state_lock. Attempting to acquire the uninitialized lock here
will cause a crash or lockdep warning.

> +			WRITE_ONCE(vcpu->arch.mp_state.mp_state, KVM_MP_STATE_STOPPED);
> +		}
[ ... ]
> diff --git a/arch/s390/kvm/arm64/inject_fault.c b/arch/s390/kvm/arm64/inject_fault.c
> new file mode 100644
> --- /dev/null
> +++ b/arch/s390/kvm/arm64/inject_fault.c
[ ... ]
> +void kvm_inject_undefined(struct kvm_vcpu *vcpu)
> +{
> +	/* Stub until s390 supports arm64 sysregs TODO sysregs*/
> +}
> +
> +int kvm_inject_sea(struct kvm_vcpu *vcpu, bool iabt, u64 addr)
> +{
> +	/* Stub until s390 supports arm64 sysregs TODO sysregs*/
> +	return 1;
> +}

[Severity: High]
Do these empty stubs cause an infinite exit loop?

If a guest executes an undefined instruction or accesses an unmapped memory 
address, the host KVM code attempts to inject an exception via 
kvm_inject_undefined() or kvm_inject_sea(). 

Because these functions are empty stubs, they do not advance the guest PC or 
alter its exception state. When the exit handler returns 1 to resume 
execution, the guest will immediately re-execute the exact same faulting 
instruction and trap again indefinitely. Any unprivileged guest process can 
trigger this to cause a denial of service against the host and the VM.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=29