head 1.2; access; symbols pkgsrc-2022Q2:1.1.0.16 pkgsrc-2022Q2-base:1.1 pkgsrc-2022Q1:1.1.0.14 pkgsrc-2022Q1-base:1.1 pkgsrc-2021Q4:1.1.0.12 pkgsrc-2021Q4-base:1.1 pkgsrc-2021Q3:1.1.0.10 pkgsrc-2021Q3-base:1.1 pkgsrc-2021Q2:1.1.0.8 pkgsrc-2021Q2-base:1.1 pkgsrc-2021Q1:1.1.0.6 pkgsrc-2021Q1-base:1.1 pkgsrc-2020Q4:1.1.0.4 pkgsrc-2020Q4-base:1.1 pkgsrc-2020Q3:1.1.0.2; locks; strict; comment @# @; 1.2 date 2022.06.28.16.33.25; author bouyer; state dead; branches; next 1.1; commitid lkljtNX5KSYPgPJD; 1.1 date 2020.10.02.13.00.48; author bouyer; state Exp; branches 1.1.2.1; next ; commitid 2MY96Iq2OdtetkqC; 1.1.2.1 date 2020.10.02.13.00.48; author bsiegert; state dead; branches; next 1.1.2.2; commitid 6fMH1kUIpq0AYCqC; 1.1.2.2 date 2020.10.04.20.44.32; author bsiegert; state Exp; branches; next ; commitid 6fMH1kUIpq0AYCqC; desc @@ 1.2 log @Remove xenkernel411 and xenkernel411, they're EOL upstream @ text @$NetBSD: patch-XSA337,v 1.1 2020/10/02 13:00:48 bouyer Exp $ From: Roger Pau Monné Subject: x86/msi: get rid of read_msi_msg It's safer and faster to just use the cached last written (untranslated) MSI message stored in msi_desc for the single user that calls read_msi_msg. This also prevents relying on the data read from the device MSI registers in order to figure out the index into the IOMMU interrupt remapping table, which is not safe. This is part of XSA-337. Reported-by: Andrew Cooper Requested-by: Andrew Cooper Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich --- xen/arch/x86/msi.c.orig +++ xen/arch/x86/msi.c @@@@ -192,59 +192,6 @@@@ void msi_compose_msg(unsigned vector, co MSI_DATA_VECTOR(vector); } -static bool read_msi_msg(struct msi_desc *entry, struct msi_msg *msg) -{ - switch ( entry->msi_attrib.type ) - { - case PCI_CAP_ID_MSI: - { - struct pci_dev *dev = entry->dev; - int pos = entry->msi_attrib.pos; - u16 data, seg = dev->seg; - u8 bus = dev->bus; - u8 slot = PCI_SLOT(dev->devfn); - u8 func = PCI_FUNC(dev->devfn); - - msg->address_lo = pci_conf_read32(seg, bus, slot, func, - msi_lower_address_reg(pos)); - if ( entry->msi_attrib.is_64 ) - { - msg->address_hi = pci_conf_read32(seg, bus, slot, func, - msi_upper_address_reg(pos)); - data = pci_conf_read16(seg, bus, slot, func, - msi_data_reg(pos, 1)); - } - else - { - msg->address_hi = 0; - data = pci_conf_read16(seg, bus, slot, func, - msi_data_reg(pos, 0)); - } - msg->data = data; - break; - } - case PCI_CAP_ID_MSIX: - { - void __iomem *base = entry->mask_base; - - if ( unlikely(!msix_memory_decoded(entry->dev, - entry->msi_attrib.pos)) ) - return false; - msg->address_lo = readl(base + PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET); - msg->address_hi = readl(base + PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET); - msg->data = readl(base + PCI_MSIX_ENTRY_DATA_OFFSET); - break; - } - default: - BUG(); - } - - if ( iommu_intremap ) - iommu_read_msi_from_ire(entry, msg); - - return true; -} - static int write_msi_msg(struct msi_desc *entry, struct msi_msg *msg) { entry->msg = *msg; @@@@ -322,10 +269,7 @@@@ void set_msi_affinity(struct irq_desc *d ASSERT(spin_is_locked(&desc->lock)); - memset(&msg, 0, sizeof(msg)); - if ( !read_msi_msg(msi_desc, &msg) ) - return; - + msg = msi_desc->msg; msg.data &= ~MSI_DATA_VECTOR_MASK; msg.data |= MSI_DATA_VECTOR(desc->arch.vector); msg.address_lo &= ~MSI_ADDR_DEST_ID_MASK; From: Jan Beulich Subject: x86/MSI-X: restrict reading of table/PBA bases from BARs When assigned to less trusted or un-trusted guests, devices may change state behind our backs (they may e.g. get reset by means we may not know about). Therefore we should avoid reading BARs from hardware once a device is no longer owned by Dom0. Furthermore when we can't read a BAR, or when we read zero, we shouldn't instead use the caller provided address unless that caller can be trusted. Re-arrange the logic in msix_capability_init() such that only Dom0 (and only if the device isn't DomU-owned yet) or calls through PHYSDEVOP_prepare_msix will actually result in the reading of the respective BAR register(s). Additionally do so only as long as in-use table entries are known (note that invocation of PHYSDEVOP_prepare_msix counts as a "pseudo" entry). In all other uses the value already recorded will get used instead. Clear the recorded values in _pci_cleanup_msix() as well as on the one affected error path. (Adjust this error path to also avoid blindly disabling MSI-X when it was enabled on entry to the function.) While moving around variable declarations (in many cases to reduce their scopes), also adjust some of their types. This is part of XSA-337. Signed-off-by: Jan Beulich Reviewed-by: Roger Pau Monné --- xen/arch/x86/msi.c.orig +++ xen/arch/x86/msi.c @@@@ -790,16 +790,14 @@@@ static int msix_capability_init(struct p { struct arch_msix *msix = dev->msix; struct msi_desc *entry = NULL; - int vf; u16 control; u64 table_paddr; u32 table_offset; - u8 bir, pbus, pslot, pfunc; u16 seg = dev->seg; u8 bus = dev->bus; u8 slot = PCI_SLOT(dev->devfn); u8 func = PCI_FUNC(dev->devfn); - bool maskall = msix->host_maskall; + bool maskall = msix->host_maskall, zap_on_error = false; ASSERT(pcidevs_locked()); @@@@ -837,43 +835,45 @@@@ static int msix_capability_init(struct p /* Locate MSI-X table region */ table_offset = pci_conf_read32(seg, bus, slot, func, msix_table_offset_reg(pos)); - bir = (u8)(table_offset & PCI_MSIX_BIRMASK); - table_offset &= ~PCI_MSIX_BIRMASK; + if ( !msix->used_entries && + (!msi || + (is_hardware_domain(current->domain) && + (dev->domain == current->domain || dev->domain == dom_io))) ) + { + unsigned int bir = table_offset & PCI_MSIX_BIRMASK, pbus, pslot, pfunc; + int vf; + paddr_t pba_paddr; + unsigned int pba_offset; - if ( !dev->info.is_virtfn ) - { - pbus = bus; - pslot = slot; - pfunc = func; - vf = -1; - } - else - { - pbus = dev->info.physfn.bus; - pslot = PCI_SLOT(dev->info.physfn.devfn); - pfunc = PCI_FUNC(dev->info.physfn.devfn); - vf = PCI_BDF2(dev->bus, dev->devfn); - } - - table_paddr = read_pci_mem_bar(seg, pbus, pslot, pfunc, bir, vf); - WARN_ON(msi && msi->table_base != table_paddr); - if ( !table_paddr ) - { - if ( !msi || !msi->table_base ) + if ( !dev->info.is_virtfn ) { - pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), - control & ~PCI_MSIX_FLAGS_ENABLE); - xfree(entry); - return -ENXIO; + pbus = bus; + pslot = slot; + pfunc = func; + vf = -1; + } + else + { + pbus = dev->info.physfn.bus; + pslot = PCI_SLOT(dev->info.physfn.devfn); + pfunc = PCI_FUNC(dev->info.physfn.devfn); + vf = PCI_BDF2(dev->bus, dev->devfn); } - table_paddr = msi->table_base; - } - table_paddr += table_offset; - if ( !msix->used_entries ) - { - u64 pba_paddr; - u32 pba_offset; + table_paddr = read_pci_mem_bar(seg, pbus, pslot, pfunc, bir, vf); + WARN_ON(msi && msi->table_base != table_paddr); + if ( !table_paddr ) + { + if ( !msi || !msi->table_base ) + { + pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), + control & ~PCI_MSIX_FLAGS_ENABLE); + xfree(entry); + return -ENXIO; + } + table_paddr = msi->table_base; + } + table_paddr += table_offset & ~PCI_MSIX_BIRMASK; msix->nr_entries = nr_entries; msix->table.first = PFN_DOWN(table_paddr); @@@@ -894,7 +894,19 @@@@ static int msix_capability_init(struct p BITS_TO_LONGS(nr_entries) - 1); WARN_ON(rangeset_overlaps_range(mmio_ro_ranges, msix->pba.first, msix->pba.last)); + + zap_on_error = true; } + else if ( !msix->table.first ) + { + pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), + control); + xfree(entry); + return -ENODATA; + } + else + table_paddr = (msix->table.first << PAGE_SHIFT) + + (table_offset & ~PCI_MSIX_BIRMASK & ~PAGE_MASK); if ( entry ) { @@@@ -905,8 +917,16 @@@@ static int msix_capability_init(struct p if ( idx < 0 ) { + if ( zap_on_error ) + { + msix->table.first = 0; + msix->pba.first = 0; + + control &= ~PCI_MSIX_FLAGS_ENABLE; + } + pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), - control & ~PCI_MSIX_FLAGS_ENABLE); + control); xfree(entry); return idx; } @@@@ -1102,9 +1122,14 @@@@ static void _pci_cleanup_msix(struct arc if ( rangeset_remove_range(mmio_ro_ranges, msix->table.first, msix->table.last) ) WARN(); + msix->table.first = 0; + msix->table.last = 0; + if ( rangeset_remove_range(mmio_ro_ranges, msix->pba.first, msix->pba.last) ) WARN(); + msix->pba.first = 0; + msix->pba.last = 0; } } @ 1.1 log @dd uptream fixes for XSA333, XSA336, XSA337, XSA338, XSA339, XSA340, XSA342, XSA343, XSA344 bump PKGREVISION @ text @d1 1 a1 1 $NetBSD: $ @ 1.1.2.1 log @file patch-XSA337 was added on branch pkgsrc-2020Q3 on 2020-10-04 20:44:32 +0000 @ text @d1 276 @ 1.1.2.2 log @Pullup ticket #6332 - requested by bouyer sysutils/xenkernel411: security fix sysutils/xenkernel413: security fix Revisions pulled up: - sysutils/xenkernel411/Makefile 1.16 - sysutils/xenkernel411/distinfo 1.14 - sysutils/xenkernel411/patches/patch-XSA333 1.1 - sysutils/xenkernel411/patches/patch-XSA336 1.1 - sysutils/xenkernel411/patches/patch-XSA337 1.1 - sysutils/xenkernel411/patches/patch-XSA338 1.1 - sysutils/xenkernel411/patches/patch-XSA339 1.1 - sysutils/xenkernel411/patches/patch-XSA340 1.1 - sysutils/xenkernel411/patches/patch-XSA342 1.1 - sysutils/xenkernel411/patches/patch-XSA343 1.1 - sysutils/xenkernel411/patches/patch-XSA344 1.1 - sysutils/xenkernel413/Makefile 1.5 - sysutils/xenkernel413/distinfo 1.3 - sysutils/xenkernel413/patches/patch-XSA333 1.1 - sysutils/xenkernel413/patches/patch-XSA334 1.1 - sysutils/xenkernel413/patches/patch-XSA336 1.1 - sysutils/xenkernel413/patches/patch-XSA337 1.1 - sysutils/xenkernel413/patches/patch-XSA338 1.1 - sysutils/xenkernel413/patches/patch-XSA339 1.1 - sysutils/xenkernel413/patches/patch-XSA340 1.1 - sysutils/xenkernel413/patches/patch-XSA342 1.1 - sysutils/xenkernel413/patches/patch-XSA343 1.1 - sysutils/xenkernel413/patches/patch-XSA344 1.1 --- Module Name: pkgsrc Committed By: bouyer Date: Thu Oct 1 12:41:19 UTC 2020 Modified Files: pkgsrc/sysutils/xenkernel413: Makefile distinfo Added Files: pkgsrc/sysutils/xenkernel413/patches: patch-XSA333 patch-XSA334 patch-XSA336 patch-XSA337 patch-XSA338 patch-XSA339 patch-XSA340 patch-XSA342 patch-XSA343 patch-XSA344 Log Message: Add uptream fixes for XSA333, XSA334, XSA336, XSA337, XSA338, XSA339, XSA340, XSA342, XSA343, XSA344 bump PKGREVISION --- Module Name: pkgsrc Committed By: bouyer Date: Fri Oct 2 13:00:48 UTC 2020 Modified Files: pkgsrc/sysutils/xenkernel411: Makefile distinfo Added Files: pkgsrc/sysutils/xenkernel411/patches: patch-XSA333 patch-XSA336 patch-XSA337 patch-XSA338 patch-XSA339 patch-XSA340 patch-XSA342 patch-XSA343 patch-XSA344 Log Message: dd uptream fixes for XSA333, XSA336, XSA337, XSA338, XSA339, XSA340, XSA342, XSA343, XSA344 bump PKGREVISION @ text @a0 276 $NetBSD: patch-XSA337,v 1.1 2020/10/02 13:00:48 bouyer Exp $ From: Roger Pau Monné Subject: x86/msi: get rid of read_msi_msg It's safer and faster to just use the cached last written (untranslated) MSI message stored in msi_desc for the single user that calls read_msi_msg. This also prevents relying on the data read from the device MSI registers in order to figure out the index into the IOMMU interrupt remapping table, which is not safe. This is part of XSA-337. Reported-by: Andrew Cooper Requested-by: Andrew Cooper Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich --- xen/arch/x86/msi.c.orig +++ xen/arch/x86/msi.c @@@@ -192,59 +192,6 @@@@ void msi_compose_msg(unsigned vector, co MSI_DATA_VECTOR(vector); } -static bool read_msi_msg(struct msi_desc *entry, struct msi_msg *msg) -{ - switch ( entry->msi_attrib.type ) - { - case PCI_CAP_ID_MSI: - { - struct pci_dev *dev = entry->dev; - int pos = entry->msi_attrib.pos; - u16 data, seg = dev->seg; - u8 bus = dev->bus; - u8 slot = PCI_SLOT(dev->devfn); - u8 func = PCI_FUNC(dev->devfn); - - msg->address_lo = pci_conf_read32(seg, bus, slot, func, - msi_lower_address_reg(pos)); - if ( entry->msi_attrib.is_64 ) - { - msg->address_hi = pci_conf_read32(seg, bus, slot, func, - msi_upper_address_reg(pos)); - data = pci_conf_read16(seg, bus, slot, func, - msi_data_reg(pos, 1)); - } - else - { - msg->address_hi = 0; - data = pci_conf_read16(seg, bus, slot, func, - msi_data_reg(pos, 0)); - } - msg->data = data; - break; - } - case PCI_CAP_ID_MSIX: - { - void __iomem *base = entry->mask_base; - - if ( unlikely(!msix_memory_decoded(entry->dev, - entry->msi_attrib.pos)) ) - return false; - msg->address_lo = readl(base + PCI_MSIX_ENTRY_LOWER_ADDR_OFFSET); - msg->address_hi = readl(base + PCI_MSIX_ENTRY_UPPER_ADDR_OFFSET); - msg->data = readl(base + PCI_MSIX_ENTRY_DATA_OFFSET); - break; - } - default: - BUG(); - } - - if ( iommu_intremap ) - iommu_read_msi_from_ire(entry, msg); - - return true; -} - static int write_msi_msg(struct msi_desc *entry, struct msi_msg *msg) { entry->msg = *msg; @@@@ -322,10 +269,7 @@@@ void set_msi_affinity(struct irq_desc *d ASSERT(spin_is_locked(&desc->lock)); - memset(&msg, 0, sizeof(msg)); - if ( !read_msi_msg(msi_desc, &msg) ) - return; - + msg = msi_desc->msg; msg.data &= ~MSI_DATA_VECTOR_MASK; msg.data |= MSI_DATA_VECTOR(desc->arch.vector); msg.address_lo &= ~MSI_ADDR_DEST_ID_MASK; From: Jan Beulich Subject: x86/MSI-X: restrict reading of table/PBA bases from BARs When assigned to less trusted or un-trusted guests, devices may change state behind our backs (they may e.g. get reset by means we may not know about). Therefore we should avoid reading BARs from hardware once a device is no longer owned by Dom0. Furthermore when we can't read a BAR, or when we read zero, we shouldn't instead use the caller provided address unless that caller can be trusted. Re-arrange the logic in msix_capability_init() such that only Dom0 (and only if the device isn't DomU-owned yet) or calls through PHYSDEVOP_prepare_msix will actually result in the reading of the respective BAR register(s). Additionally do so only as long as in-use table entries are known (note that invocation of PHYSDEVOP_prepare_msix counts as a "pseudo" entry). In all other uses the value already recorded will get used instead. Clear the recorded values in _pci_cleanup_msix() as well as on the one affected error path. (Adjust this error path to also avoid blindly disabling MSI-X when it was enabled on entry to the function.) While moving around variable declarations (in many cases to reduce their scopes), also adjust some of their types. This is part of XSA-337. Signed-off-by: Jan Beulich Reviewed-by: Roger Pau Monné --- xen/arch/x86/msi.c.orig +++ xen/arch/x86/msi.c @@@@ -790,16 +790,14 @@@@ static int msix_capability_init(struct p { struct arch_msix *msix = dev->msix; struct msi_desc *entry = NULL; - int vf; u16 control; u64 table_paddr; u32 table_offset; - u8 bir, pbus, pslot, pfunc; u16 seg = dev->seg; u8 bus = dev->bus; u8 slot = PCI_SLOT(dev->devfn); u8 func = PCI_FUNC(dev->devfn); - bool maskall = msix->host_maskall; + bool maskall = msix->host_maskall, zap_on_error = false; ASSERT(pcidevs_locked()); @@@@ -837,43 +835,45 @@@@ static int msix_capability_init(struct p /* Locate MSI-X table region */ table_offset = pci_conf_read32(seg, bus, slot, func, msix_table_offset_reg(pos)); - bir = (u8)(table_offset & PCI_MSIX_BIRMASK); - table_offset &= ~PCI_MSIX_BIRMASK; + if ( !msix->used_entries && + (!msi || + (is_hardware_domain(current->domain) && + (dev->domain == current->domain || dev->domain == dom_io))) ) + { + unsigned int bir = table_offset & PCI_MSIX_BIRMASK, pbus, pslot, pfunc; + int vf; + paddr_t pba_paddr; + unsigned int pba_offset; - if ( !dev->info.is_virtfn ) - { - pbus = bus; - pslot = slot; - pfunc = func; - vf = -1; - } - else - { - pbus = dev->info.physfn.bus; - pslot = PCI_SLOT(dev->info.physfn.devfn); - pfunc = PCI_FUNC(dev->info.physfn.devfn); - vf = PCI_BDF2(dev->bus, dev->devfn); - } - - table_paddr = read_pci_mem_bar(seg, pbus, pslot, pfunc, bir, vf); - WARN_ON(msi && msi->table_base != table_paddr); - if ( !table_paddr ) - { - if ( !msi || !msi->table_base ) + if ( !dev->info.is_virtfn ) { - pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), - control & ~PCI_MSIX_FLAGS_ENABLE); - xfree(entry); - return -ENXIO; + pbus = bus; + pslot = slot; + pfunc = func; + vf = -1; + } + else + { + pbus = dev->info.physfn.bus; + pslot = PCI_SLOT(dev->info.physfn.devfn); + pfunc = PCI_FUNC(dev->info.physfn.devfn); + vf = PCI_BDF2(dev->bus, dev->devfn); } - table_paddr = msi->table_base; - } - table_paddr += table_offset; - if ( !msix->used_entries ) - { - u64 pba_paddr; - u32 pba_offset; + table_paddr = read_pci_mem_bar(seg, pbus, pslot, pfunc, bir, vf); + WARN_ON(msi && msi->table_base != table_paddr); + if ( !table_paddr ) + { + if ( !msi || !msi->table_base ) + { + pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), + control & ~PCI_MSIX_FLAGS_ENABLE); + xfree(entry); + return -ENXIO; + } + table_paddr = msi->table_base; + } + table_paddr += table_offset & ~PCI_MSIX_BIRMASK; msix->nr_entries = nr_entries; msix->table.first = PFN_DOWN(table_paddr); @@@@ -894,7 +894,19 @@@@ static int msix_capability_init(struct p BITS_TO_LONGS(nr_entries) - 1); WARN_ON(rangeset_overlaps_range(mmio_ro_ranges, msix->pba.first, msix->pba.last)); + + zap_on_error = true; } + else if ( !msix->table.first ) + { + pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), + control); + xfree(entry); + return -ENODATA; + } + else + table_paddr = (msix->table.first << PAGE_SHIFT) + + (table_offset & ~PCI_MSIX_BIRMASK & ~PAGE_MASK); if ( entry ) { @@@@ -905,8 +917,16 @@@@ static int msix_capability_init(struct p if ( idx < 0 ) { + if ( zap_on_error ) + { + msix->table.first = 0; + msix->pba.first = 0; + + control &= ~PCI_MSIX_FLAGS_ENABLE; + } + pci_conf_write16(seg, bus, slot, func, msix_control_reg(pos), - control & ~PCI_MSIX_FLAGS_ENABLE); + control); xfree(entry); return idx; } @@@@ -1102,9 +1122,14 @@@@ static void _pci_cleanup_msix(struct arc if ( rangeset_remove_range(mmio_ro_ranges, msix->table.first, msix->table.last) ) WARN(); + msix->table.first = 0; + msix->table.last = 0; + if ( rangeset_remove_range(mmio_ro_ranges, msix->pba.first, msix->pba.last) ) WARN(); + msix->pba.first = 0; + msix->pba.last = 0; } } @