Re: [PATCH v2 1/2] KVM: s390: Limit adapter indicator access to mapped page

[Christian Borntraeger <borntraeger@linux.ibm.com>]

Am 03.03.26 um 14:46 schrieb Janosch Frank:
> While we check the address for errors, we don't seem to check the bit
> offsets and since they are 32 and 64 bits a lot of memory can be
> reached indirectly via those offsets.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Fixes: 84223598778b ("KVM: s390: irq routing for adapter interrupts.")
> Suggested-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>

> ---
>   arch/s390/kvm/interrupt.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 18932a65ca68..1a702e8ef574 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -2724,6 +2724,9 @@ static unsigned long get_ind_bit(__u64 addr, unsigned long bit_nr, bool swap)
>   
>   	bit = bit_nr + (addr % PAGE_SIZE) * 8;
>   
> +	/* kvm_set_routing_entry() should never allow this to happen */
> +	WARN_ON_ONCE(bit > (PAGE_SIZE * BITS_PER_BYTE - 1));
> +
>   	return swap ? (bit ^ (BITS_PER_LONG - 1)) : bit;
>   }
>   
> @@ -2852,6 +2855,7 @@ int kvm_set_routing_entry(struct kvm *kvm,
>   			  struct kvm_kernel_irq_routing_entry *e,
>   			  const struct kvm_irq_routing_entry *ue)
>   {
> +	const struct kvm_irq_routing_s390_adapter *adapter;
>   	u64 uaddr_s, uaddr_i;
>   	int idx;
>   
> @@ -2862,6 +2866,14 @@ int kvm_set_routing_entry(struct kvm *kvm,
>   			return -EINVAL;
>   		e->set = set_adapter_int;
>   
> +		adapter = &ue->u.adapter;
> +		if (adapter->summary_addr + (adapter->summary_offset / 8) >=
> +		    (adapter->summary_addr & PAGE_MASK) + PAGE_SIZE)
> +			return -EINVAL;
> +		if (adapter->ind_addr + (adapter->ind_offset / 8) >=
> +		    (adapter->ind_addr & PAGE_MASK) + PAGE_SIZE)
> +			return -EINVAL;
> +
>   		idx = srcu_read_lock(&kvm->srcu);
>   		uaddr_s = gpa_to_hva(kvm, ue->u.adapter.summary_addr);
>   		uaddr_i = gpa_to_hva(kvm, ue->u.adapter.ind_addr);

we could consider a follow-on patch to use the local adapter variable here
and below as well. But not as part of this fix.