* [PATCH 0/2] KVM: s390: Limit adapter indicator access to page @ 2026-02-17 8:54 Janosch Frank 2026-02-17 8:54 ` [PATCH 1/2] KVM: s390: Limit adapter indicator access to mapped page Janosch Frank 2026-02-17 8:54 ` [PATCH 2/2] KVM: s390: selftests: Add IRQ routing address offset tests Janosch Frank 0 siblings, 2 replies; 7+ messages in thread From: Janosch Frank @ 2026-02-17 8:54 UTC (permalink / raw) To: kvm; +Cc: linux-s390, imbrenda, borntraeger, freimuth, mjrosato 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] KVM: s390: Limit adapter indicator access to mapped page 2026-02-17 8:54 [PATCH 0/2] KVM: s390: Limit adapter indicator access to page Janosch Frank @ 2026-02-17 8:54 ` Janosch Frank 2026-02-25 23:42 ` Matthew Rosato 2026-02-26 15:59 ` Claudio Imbrenda 2026-02-17 8:54 ` [PATCH 2/2] KVM: s390: selftests: Add IRQ routing address offset tests Janosch Frank 1 sibling, 2 replies; 7+ messages in thread From: Janosch Frank @ 2026-02-17 8:54 UTC (permalink / raw) To: kvm; +Cc: linux-s390, imbrenda, borntraeger, freimuth, mjrosato 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] KVM: s390: Limit adapter indicator access to mapped page 2026-02-17 8:54 ` [PATCH 1/2] KVM: s390: Limit adapter indicator access to mapped page Janosch Frank @ 2026-02-25 23:42 ` Matthew Rosato 2026-02-26 14:03 ` Janosch Frank 2026-02-26 15:59 ` Claudio Imbrenda 1 sibling, 1 reply; 7+ messages in thread From: Matthew Rosato @ 2026-02-25 23:42 UTC (permalink / raw) To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, borntraeger, freimuth 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); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] KVM: s390: Limit adapter indicator access to mapped page 2026-02-25 23:42 ` Matthew Rosato @ 2026-02-26 14:03 ` Janosch Frank 0 siblings, 0 replies; 7+ messages in thread From: Janosch Frank @ 2026-02-26 14:03 UTC (permalink / raw) To: Matthew Rosato, kvm; +Cc: linux-s390, imbrenda, borntraeger, freimuth 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] KVM: s390: Limit adapter indicator access to mapped page 2026-02-17 8:54 ` [PATCH 1/2] KVM: s390: Limit adapter indicator access to mapped page Janosch Frank 2026-02-25 23:42 ` Matthew Rosato @ 2026-02-26 15:59 ` Claudio Imbrenda 2026-02-27 12:14 ` Janosch Frank 1 sibling, 1 reply; 7+ messages in thread From: Claudio Imbrenda @ 2026-02-26 15:59 UTC (permalink / raw) To: Janosch Frank; +Cc: kvm, linux-s390, borntraeger, freimuth, mjrosato 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); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] KVM: s390: Limit adapter indicator access to mapped page 2026-02-26 15:59 ` Claudio Imbrenda @ 2026-02-27 12:14 ` Janosch Frank 0 siblings, 0 replies; 7+ messages in thread From: Janosch Frank @ 2026-02-27 12:14 UTC (permalink / raw) To: Claudio Imbrenda; +Cc: kvm, linux-s390, borntraeger, freimuth, mjrosato 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] KVM: s390: selftests: Add IRQ routing address offset tests 2026-02-17 8:54 [PATCH 0/2] KVM: s390: Limit adapter indicator access to page Janosch Frank 2026-02-17 8:54 ` [PATCH 1/2] KVM: s390: Limit adapter indicator access to mapped page Janosch Frank @ 2026-02-17 8:54 ` Janosch Frank 1 sibling, 0 replies; 7+ messages in thread From: Janosch Frank @ 2026-02-17 8:54 UTC (permalink / raw) To: kvm; +Cc: linux-s390, imbrenda, borntraeger, freimuth, mjrosato 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-02-27 12:14 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-02-17 8:54 [PATCH 0/2] KVM: s390: Limit adapter indicator access to page Janosch Frank 2026-02-17 8:54 ` [PATCH 1/2] KVM: s390: Limit adapter indicator access to mapped page Janosch Frank 2026-02-25 23:42 ` Matthew Rosato 2026-02-26 14:03 ` Janosch Frank 2026-02-26 15:59 ` Claudio Imbrenda 2026-02-27 12:14 ` Janosch Frank 2026-02-17 8:54 ` [PATCH 2/2] KVM: s390: selftests: Add IRQ routing address offset tests Janosch Frank
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox