From: Jan Beulich Subject: x86/HVM: add locking to I/O port translation list traversal 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Ă© --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -667,6 +667,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 ) { @@ -688,11 +689,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); } } @@ -701,6 +705,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 ) { @@ -708,6 +714,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 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -160,7 +160,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); } --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -613,6 +613,7 @@ int hvm_domain_initialise(struct domain spin_lock_init(&d->arch.hvm.irq_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); --- a/xen/arch/x86/hvm/io.c +++ b/xen/arch/x86/hvm/io.c @@ -143,36 +143,56 @@ bool handle_pio(uint16_t port, unsigned 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 ) { @@ -189,6 +209,8 @@ static int cf_check g2m_portio_read( BUG(); } + read_unlock(&hvm->g2m_ioport_lock); + return X86EMUL_OKAY; } @@ -196,9 +218,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 ) { @@ -215,6 +243,8 @@ static int cf_check g2m_portio_write( BUG(); } + read_unlock(&hvm->g2m_ioport_lock); + return X86EMUL_OKAY; } --- a/xen/arch/x86/include/asm/hvm/domain.h +++ b/xen/arch/x86/include/asm/hvm/domain.h @@ -124,6 +124,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; --- a/xen/arch/x86/include/asm/hvm/vcpu.h +++ b/xen/arch/x86/include/asm/hvm/vcpu.h @@ -53,8 +53,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 {