[PATCH 0/2] KVM: s390: Limit adapter indicator access to page

[PATCH 0/2] KVM: s390: Limit adapter indicator access to page

[Janosch Frank]

We currently check the address of the indicator fields but not the sum
of address and offset. This patch remedies that problem and limits the
address + offset combination to a single page.

The selftest is very rudimentary but it's a start.

Janosch Frank (2):
  KVM: s390: Limit adapter indicator access to mapped page
  KVM: s390: selftests: Add IRQ routing address offset tests

 arch/s390/kvm/interrupt.c                     | 12 +++
 tools/testing/selftests/kvm/Makefile.kvm      |  1 +
 .../testing/selftests/kvm/s390/irq_routing.c  | 75 +++++++++++++++++++
 3 files changed, 88 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/s390/irq_routing.c

-- 
2.53.0

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

[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.")
---
 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 1c2bb5cd7e12..cd4851e33a5b 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 + BITS_TO_BYTES(adapter->summary_offset) >=
+		    (adapter->summary_addr & PAGE_MASK) + PAGE_SIZE)
+			return -EINVAL;
+		if (adapter->ind_addr + BITS_TO_BYTES(adapter->ind_offset) >=
+		    (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);
-- 
2.53.0

[PATCH 2/2] KVM: s390: selftests: Add IRQ routing address offset tests

[Janosch Frank]

This test tries to setup routes which have address + offset
combinations which cross a page.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |  1 +
 .../testing/selftests/kvm/s390/irq_routing.c  | 75 +++++++++++++++++++
 2 files changed, 76 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/s390/irq_routing.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 7cfdfe7edfbf..c757704a0cb7 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -201,6 +201,7 @@ TEST_GEN_PROGS_s390 += s390/ucontrol_test
 TEST_GEN_PROGS_s390 += s390/user_operexec
 TEST_GEN_PROGS_s390 += s390/keyop
 TEST_GEN_PROGS_s390 += rseq_test
+TEST_GEN_PROGS_s390 += s390/irq_routing
 
 TEST_GEN_PROGS_riscv = $(TEST_GEN_PROGS_COMMON)
 TEST_GEN_PROGS_riscv += riscv/sbi_pmu_test
diff --git a/tools/testing/selftests/kvm/s390/irq_routing.c b/tools/testing/selftests/kvm/s390/irq_routing.c
new file mode 100644
index 000000000000..4d9b5df2e456
--- /dev/null
+++ b/tools/testing/selftests/kvm/s390/irq_routing.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * IRQ routing offset tests.
+ *
+ * Copyright IBM Corp. 2026
+ *
+ * Authors:
+ *  Janosch Frank <frankja@linux.ibm.com>
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "kselftest.h"
+#include "ucall_common.h"
+
+extern char guest_code[];
+asm("guest_code:\n"
+    "diag %r0,%r0,0\n"
+    "j .\n");
+
+static void test(void)
+{
+	struct kvm_irq_routing *routing;
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	vm_paddr_t mem;
+	int ret;
+
+	struct kvm_irq_routing_entry ue = {
+		.type = KVM_IRQ_ROUTING_S390_ADAPTER,
+		.gsi = 1,
+	};
+
+	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+	mem = vm_phy_pages_alloc(vm, 2, 4096 * 42, 0);
+
+	routing = kvm_gsi_routing_create();
+	routing->nr = 1;
+	routing->entries[0] = ue;
+	routing->entries[0].u.adapter.summary_addr = (uintptr_t)mem;
+	routing->entries[0].u.adapter.ind_addr = (uintptr_t)mem;
+
+	routing->entries[0].u.adapter.summary_offset = 4096 * 8;
+	ret = __vm_ioctl(vm, KVM_SET_GSI_ROUTING, routing);
+	ksft_test_result(ret == -1 && errno == EINVAL, "summary offset outside of page\n");
+
+	routing->entries[0].u.adapter.summary_offset -= 8;
+	ret = __vm_ioctl(vm, KVM_SET_GSI_ROUTING, routing);
+	ksft_test_result(ret == 0, "summary offset inside of page\n");
+
+	routing->entries[0].u.adapter.ind_offset = 4096 * 8;
+	ret = __vm_ioctl(vm, KVM_SET_GSI_ROUTING, routing);
+	ksft_test_result(ret == -1 && errno == EINVAL, "ind offset outside of page\n");
+
+	routing->entries[0].u.adapter.ind_offset -= 8;
+	ret = __vm_ioctl(vm, KVM_SET_GSI_ROUTING, routing);
+	ksft_test_result(ret == 0, "ind offset inside of page\n");
+
+	kvm_vm_free(vm);
+}
+
+int main(int argc, char *argv[])
+{
+	TEST_REQUIRE(kvm_has_cap(KVM_CAP_IRQ_ROUTING));
+
+	ksft_print_header();
+	ksft_set_plan(4);
+	test();
+
+	ksft_finished();	/* Print results and exit() accordingly */
+}
-- 
2.53.0

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

[Matthew Rosato]

On 2/17/26 3:54 AM, Janosch Frank wrote:
> 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.")
> ---
>  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 1c2bb5cd7e12..cd4851e33a5b 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 + BITS_TO_BYTES(adapter->summary_offset) >=
> +		    (adapter->summary_addr & PAGE_MASK) + PAGE_SIZE)
> +			return -EINVAL;
> +		if (adapter->ind_addr + BITS_TO_BYTES(adapter->ind_offset) >=
> +		    (adapter->ind_addr & PAGE_MASK) + PAGE_SIZE)
> +			return -EINVAL;
> +


I think this is slightly off.

The offset should indicate a bit offset from the beginning of the byte, so offsets 0-7 are all within the same byte as the specified address, 8-15 are in the next byte, etc.

I hacked QEMU and tested something like...
1) addr 8126efff offset 7 -- this would be the very last bit in the page.
2) addr 8126efff offset 8 -- this would be the very first bit in the next page.  
3) addr 8126efff offset 9 -- this would be the 2nd bit in the next page.

I expected (1) to pass while (2) and (3) were rejected, but all 3 were rejected by your check.

I think the problem is that BITS_TO_BYTES rounds up.  So:
BITS_TO_BYTES(0) = 0
BITS_TO_BYTES(1..8) = 1
BITS_TO_BYTES(9..16) = 2
and so on.

But your offset check expects
0..7 = 0
8..15 = 1
and so on.

AFAICT replacing BITS_TO_BYTES(offset) with (offset / 8) would work.


>  		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);

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

[Janosch Frank]

On 2/26/26 00:42, Matthew Rosato wrote:
> On 2/17/26 3:54 AM, Janosch Frank wrote:
>> 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.")
>> ---
>>   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 1c2bb5cd7e12..cd4851e33a5b 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 + BITS_TO_BYTES(adapter->summary_offset) >=
>> +		    (adapter->summary_addr & PAGE_MASK) + PAGE_SIZE)
>> +			return -EINVAL;
>> +		if (adapter->ind_addr + BITS_TO_BYTES(adapter->ind_offset) >=
>> +		    (adapter->ind_addr & PAGE_MASK) + PAGE_SIZE)
>> +			return -EINVAL;
>> +
> 
> 
> I think this is slightly off.
> 
> The offset should indicate a bit offset from the beginning of the byte, so offsets 0-7 are all within the same byte as the specified address, 8-15 are in the next byte, etc.
> 
> I hacked QEMU and tested something like...
> 1) addr 8126efff offset 7 -- this would be the very last bit in the page.
> 2) addr 8126efff offset 8 -- this would be the very first bit in the next page.
> 3) addr 8126efff offset 9 -- this would be the 2nd bit in the next page.
> 
> I expected (1) to pass while (2) and (3) were rejected, but all 3 were rejected by your check.
> 
> I think the problem is that BITS_TO_BYTES rounds up.  So:
> BITS_TO_BYTES(0) = 0

ugh, right.


> BITS_TO_BYTES(1..8) = 1
> BITS_TO_BYTES(9..16) = 2
> and so on.
> 
> But your offset check expects
> 0..7 = 0
> 8..15 = 1
> and so on.
> 
> AFAICT replacing BITS_TO_BYTES(offset) with (offset / 8) would work.

Yeah, rounding down should work.
I wanted to have a fancy macro but didn't work out...

I'll re-spin and make sure the test will catch this problem as well as 
the issue I'm trying to fix.

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

[Claudio Imbrenda]

On Tue, 17 Feb 2026 09:54:22 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:

> 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>
Reported-by: Claudio Imbrenda <imbrenda@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 1c2bb5cd7e12..cd4851e33a5b 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 + BITS_TO_BYTES(adapter->summary_offset) >=
> +		    (adapter->summary_addr & PAGE_MASK) + PAGE_SIZE)
> +			return -EINVAL;
> +		if (adapter->ind_addr + BITS_TO_BYTES(adapter->ind_offset) >=
> +		    (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);

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

[Janosch Frank]

On 2/26/26 16:59, Claudio Imbrenda wrote:
> On Tue, 17 Feb 2026 09:54:22 +0100
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> 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>
> Reported-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> 

Just spoke with Claudio, he confused this patch with another one.

Since he suggested a fix after I reported the issue to Christian and him 
I'll keep the suggested-by though.