Re: [PATCH v10 3/3] KVM: s390: Introducing kvm_arch_set_irq_inatomic fast inject

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Missing KVM Dirty Page Tracking during Fast IRQ Injection.

Pre-existing issues:
- [High] Guest-Triggerable WARNING / Denial of Service on memory allocation failure.
--

commit 87294ba489c0602b2b9baebf0c40ddd4f77205c0
Author: Douglas Freimuth <freimuth@linux.ibm.com>
Subject: KVM: s390: Introducing kvm_arch_set_irq_inatomic fast inject

This patch introduces a fast path for irq injection on s390 via
kvm_arch_set_irq_inatomic. It avoids placing interrupts on the global
work queue by directly mapping adapter indicator pages and accessing
shared resources using spinlocks instead of mutexes.

> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index b5304816aaa04..9e3e6b0d72ad6 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
[ ... ]
> @@ -2969,6 +2998,44 @@ static int adapter_indicators_set(struct kvm *kvm,
>  	return summary_set ? 0 : 1;
>  }
>  
> +static int adapter_indicators_set_fast(struct kvm *kvm,
> +				       struct s390_io_adapter *adapter,
> +				       struct kvm_s390_adapter_int *adapter_int,
> +				       int setbit)
> +{
> +	unsigned long bit;
> +	int summary_set;
> +	struct s390_map_info *ind_info, *summary_info;
> +	void *map;
> +
> +	spin_lock(&adapter->maps_lock);
> +	ind_info = get_map_info(adapter, adapter_int->ind_addr);
> +	if (!ind_info) {
> +		spin_unlock(&adapter->maps_lock);
> +		return -EWOULDBLOCK;
> +	}
> +	map = page_address(ind_info->page);
> +	bit = get_ind_bit(ind_info->addr, adapter_int->ind_offset, adapter->swap);
> +	if (setbit)
> +		set_bit(bit, map);

[Severity: High]
Does this bypass dirty page tracking? Since the guest memory is directly
mutated here using the kernel virtual address mapping, the hardware dirty
bit won't be set.

It appears KVM must explicitly invoke mark_page_dirty() (or an equivalent) to
record this modification in the memory slot's dirty bitmap. Without it, pending
interrupts injected via the fast path might be lost after live migration,
leading to guest I/O hangs or data corruption.

> +	summary_info = get_map_info(adapter, adapter_int->summary_addr);
> +	if (!summary_info) {
> +		spin_unlock(&adapter->maps_lock);
> +		return -EWOULDBLOCK;
> +	}
> +	map = page_address(summary_info->page);
> +	bit = get_ind_bit(summary_info->addr, adapter_int->summary_offset,
> +			  adapter->swap);
> +	/* If setbit then set summary bit. Else if falling back to the slow path */
> +	/* with setbit==0 then clear the summary bit so the slow path re-injects */
> +	if (setbit)
> +		summary_set = test_and_set_bit(bit, map);
> +	else
> +		summary_set = test_and_clear_bit(bit, map);

[Severity: High]
Similarly, does the summary page modification here also need explicit dirty 
page tracking?

> +	spin_unlock(&adapter->maps_lock);
> +	return summary_set ? 0 : 1;
> +}
[ ... ]
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 0d39c1375de21..98e7d807d6204 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
[ ... ]
> @@ -4379,19 +4389,28 @@ int kvm_s390_try_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clo
>  }
>  
>  static void __kvm_inject_pfault_token(struct kvm_vcpu *vcpu, bool start_token,
> -				      unsigned long token)
> +				     unsigned long token)
>  {
>  	struct kvm_s390_interrupt inti;
>  	struct kvm_s390_irq irq;
> +	struct kvm_s390_interrupt_info *inti_mem = NULL;
> +	int ret = 0;
>  
>  	if (start_token) {
>  		irq.u.ext.ext_params2 = token;
>  		irq.type = KVM_S390_INT_PFAULT_INIT;
>  		WARN_ON_ONCE(kvm_s390_inject_vcpu(vcpu, &irq));
>  	} else {
> +		inti_mem = kzalloc_obj(*inti_mem, GFP_KERNEL_ACCOUNT);
> +		if (WARN_ON_ONCE(!inti_mem))
> +			return;

[Severity: High]
This isn't a bug introduced by this patch, but does this guest-triggerable
warning create a Denial of Service risk?

The KVM subsystem guidelines explicitly state there should be no guest- or 
host-userspace-reachable WARN_ON or BUG_ON. If this allocation fails (e.g. 
under host memory pressure, or if a malicious guest deliberately exhausts its 
memory cgroup limits via asynchronous page faults), it will hit this warning. 
On systems configured with panic_on_warn=1, an unprivileged guest could 
intentionally crash the host.

> +
>  		inti.type = KVM_S390_INT_PFAULT_DONE;
>  		inti.parm64 = token;
> -		WARN_ON_ONCE(kvm_s390_inject_vm(vcpu->kvm, &inti));
> +		ret = kvm_s390_inject_vm(vcpu->kvm, &inti, inti_mem);
> +		if (ret)
> +			kfree(inti_mem);
> +		WARN_ON_ONCE(ret);
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604192755.203143-1-freimuth@linux.ibm.com?part=3