From 4ef23e64ebf9ef5d22bdb53b8ab418ee18a49037 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Thu, 4 Jun 2026 21:41:38 +0100 Subject: [PATCH] x86/HVM: add locking to I/O port translation list traversal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit XEN_DOMCTL_ioport_mapping is usable by DM stubdoms, and hence we can't assume the list to be left unaltered while the guest (really: the hypervisor on behalf of the guest) is accessing it. This is XSA-491 / CVE-2026-42487. Fixes: 192c4dabc344 ("domctl and p2m changes for PCI passthru") Signed-off-by: Jan Beulich Reviewed-by: Roger Pau MonnĂ© (cherry picked from commit 7787f8e9396d06026f72d3db13e836f365e085f8) --- xen/arch/x86/domctl.c | 8 +++ xen/arch/x86/hvm/emulate.c | 1 - xen/arch/x86/hvm/hvm.c | 1 + xen/arch/x86/hvm/io.c | 72 +++++++++++++++++++-------- xen/arch/x86/include/asm/hvm/domain.h | 1 + xen/arch/x86/include/asm/hvm/vcpu.h | 2 - 6 files changed, 61 insertions(+), 24 deletions(-) diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index 9bb90a83cf..877a6767a0 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -619,6 +619,7 @@ long arch_do_domctl( "ioport_map:add: dom%d gport=%x mport=%x nr=%x\n", d->domain_id, fgp, fmp, np); + write_lock(&hvm->g2m_ioport_lock); list_for_each_entry(g2m_ioport, &hvm->g2m_ioport_list, list) if (g2m_ioport->mport == fmp ) { @@ -640,11 +641,14 @@ long arch_do_domctl( g2m_ioport->np = np; list_add_tail(&g2m_ioport->list, &hvm->g2m_ioport_list); } + write_unlock(&hvm->g2m_ioport_lock); if ( !ret ) ret = ioports_permit_access(d, fmp, fmp + np - 1); if ( ret && !found && g2m_ioport ) { + write_lock(&hvm->g2m_ioport_lock); list_del(&g2m_ioport->list); + write_unlock(&hvm->g2m_ioport_lock); xfree(g2m_ioport); } } @@ -653,6 +657,8 @@ long arch_do_domctl( printk(XENLOG_G_INFO "ioport_map:remove: dom%d gport=%x mport=%x nr=%x\n", d->domain_id, fgp, fmp, np); + + write_lock(&hvm->g2m_ioport_lock); list_for_each_entry(g2m_ioport, &hvm->g2m_ioport_list, list) if ( g2m_ioport->mport == fmp ) { @@ -660,6 +666,8 @@ long arch_do_domctl( xfree(g2m_ioport); break; } + write_unlock(&hvm->g2m_ioport_lock); + ret = ioports_deny_access(d, fmp, fmp + np - 1); if ( ret && is_hardware_domain(currd) ) printk(XENLOG_ERR diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index aa77466a77..af443555cc 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -157,7 +157,6 @@ void hvmemul_cancel(struct vcpu *v) hvio->mmio_insn_bytes = 0; hvio->mmio_access = (struct npfec){}; hvio->mmio_retry = false; - hvio->g2m_ioport = NULL; hvmemul_cache_disable(v); } diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 3662e23cb6..f679fc0a44 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -596,6 +596,7 @@ int hvm_domain_initialise(struct domain *d, spin_lock_init(&d->arch.hvm.irq_lock); spin_lock_init(&d->arch.hvm.uc_lock); spin_lock_init(&d->arch.hvm.write_map.lock); + rwlock_init(&d->arch.hvm.g2m_ioport_lock); rwlock_init(&d->arch.hvm.mmcfg_lock); INIT_LIST_HEAD(&d->arch.hvm.write_map.list); INIT_LIST_HEAD(&d->arch.hvm.g2m_ioport_list); diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c index d75af83ad0..1d676833c7 100644 --- a/xen/arch/x86/hvm/io.c +++ b/xen/arch/x86/hvm/io.c @@ -145,36 +145,56 @@ bool handle_pio(uint16_t port, unsigned int size, int dir) return true; } -static bool cf_check g2m_portio_accept( - const struct hvm_io_handler *handler, const ioreq_t *p) +/* NB: Returns with the lock held in the success case. */ +static const struct g2m_ioport *g2m_portio_find_and_lock(struct hvm_domain *hvm, + uint64_t addr, + uint32_t size) { - struct vcpu *curr = current; - const struct hvm_domain *hvm = &curr->domain->arch.hvm; - struct hvm_vcpu_io *hvio = &curr->arch.hvm.hvm_io; - struct g2m_ioport *g2m_ioport; - unsigned int start, end; + const struct g2m_ioport *g2m_ioport; + + read_lock(&hvm->g2m_ioport_lock); list_for_each_entry( g2m_ioport, &hvm->g2m_ioport_list, list ) { - start = g2m_ioport->gport; - end = start + g2m_ioport->np; - if ( (p->addr >= start) && (p->addr + p->size <= end) ) - { - hvio->g2m_ioport = g2m_ioport; - return 1; - } + unsigned int start = g2m_ioport->gport; + + if ( addr >= start && addr + size <= start + g2m_ioport->np ) + return g2m_ioport; } - return 0; + read_unlock(&hvm->g2m_ioport_lock); + + return NULL; +} + +static bool cf_check g2m_portio_accept( + const struct hvm_io_handler *handler, const ioreq_t *p) +{ + struct hvm_domain *hvm = ¤t->domain->arch.hvm; + const struct g2m_ioport *g2m_ioport = + g2m_portio_find_and_lock(hvm, p->addr, p->size); + + if ( !g2m_ioport ) + return false; + + read_unlock(&hvm->g2m_ioport_lock); + + return true; } static int cf_check g2m_portio_read( const struct hvm_io_handler *handler, uint64_t addr, uint32_t size, uint64_t *data) { - struct hvm_vcpu_io *hvio = ¤t->arch.hvm.hvm_io; - const struct g2m_ioport *g2m_ioport = hvio->g2m_ioport; - unsigned int mport = (addr - g2m_ioport->gport) + g2m_ioport->mport; + struct hvm_domain *hvm = ¤t->domain->arch.hvm; + const struct g2m_ioport *g2m_ioport = + g2m_portio_find_and_lock(hvm, addr, size); + unsigned int mport; + + if ( !g2m_ioport ) + return X86EMUL_RETRY; + + mport = addr - g2m_ioport->gport + g2m_ioport->mport; switch ( size ) { @@ -191,6 +211,8 @@ static int cf_check g2m_portio_read( BUG(); } + read_unlock(&hvm->g2m_ioport_lock); + return X86EMUL_OKAY; } @@ -198,9 +220,15 @@ static int cf_check g2m_portio_write( const struct hvm_io_handler *handler, uint64_t addr, uint32_t size, uint64_t data) { - struct hvm_vcpu_io *hvio = ¤t->arch.hvm.hvm_io; - const struct g2m_ioport *g2m_ioport = hvio->g2m_ioport; - unsigned int mport = (addr - g2m_ioport->gport) + g2m_ioport->mport; + struct hvm_domain *hvm = ¤t->domain->arch.hvm; + const struct g2m_ioport *g2m_ioport = + g2m_portio_find_and_lock(hvm, addr, size); + unsigned int mport; + + if ( !g2m_ioport ) + return X86EMUL_RETRY; + + mport = addr - g2m_ioport->gport + g2m_ioport->mport; switch ( size ) { @@ -217,6 +245,8 @@ static int cf_check g2m_portio_write( BUG(); } + read_unlock(&hvm->g2m_ioport_lock); + return X86EMUL_OKAY; } diff --git a/xen/arch/x86/include/asm/hvm/domain.h b/xen/arch/x86/include/asm/hvm/domain.h index 333501d5f2..1b2c59b18b 100644 --- a/xen/arch/x86/include/asm/hvm/domain.h +++ b/xen/arch/x86/include/asm/hvm/domain.h @@ -125,6 +125,7 @@ struct hvm_domain { /* List of guest to machine IO ports mapping. */ struct list_head g2m_ioport_list; + rwlock_t g2m_ioport_lock; /* List of MMCFG regions trapped by Xen. */ struct list_head mmcfg_regions; diff --git a/xen/arch/x86/include/asm/hvm/vcpu.h b/xen/arch/x86/include/asm/hvm/vcpu.h index 3985994298..4e61b87df3 100644 --- a/xen/arch/x86/include/asm/hvm/vcpu.h +++ b/xen/arch/x86/include/asm/hvm/vcpu.h @@ -54,8 +54,6 @@ struct hvm_vcpu_io { unsigned long msix_unmask_address; unsigned long msix_snoop_address; unsigned long msix_snoop_gpa; - - const struct g2m_ioport *g2m_ioport; }; struct nestedvcpu {