[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen stable-4.11] x86/MSI-X: restrict reading of table/PBA bases from BARs



commit 2fe163d70f3ae88c10f50d70a50513e395326a77
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Tue Sep 22 17:17:47 2020 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Sep 22 17:17:47 2020 +0200

    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 <jbeulich@xxxxxxxx>
    Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 xen/arch/x86/msi.c | 97 ++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 61 insertions(+), 36 deletions(-)

diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 925ec710f4..5e5cf83c10 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -785,16 +785,14 @@ static int msix_capability_init(struct pci_dev *dev,
 {
     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());
 
@@ -832,43 +830,45 @@ static int msix_capability_init(struct pci_dev *dev,
     /* 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 ( !dev->info.is_virtfn )
-    {
-        pbus = bus;
-        pslot = slot;
-        pfunc = func;
-        vf = -1;
-    }
-    else
+    if ( !msix->used_entries &&
+         (!msi ||
+          (is_hardware_domain(current->domain) &&
+           (dev->domain == current->domain || dev->domain == dom_io))) )
     {
-        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);
-    }
+        unsigned int bir = table_offset & PCI_MSIX_BIRMASK, pbus, pslot, pfunc;
+        int vf;
+        paddr_t pba_paddr;
+        unsigned int 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 )
+        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);
@@ -889,7 +889,19 @@ static int msix_capability_init(struct pci_dev *dev,
                                   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 )
     {
@@ -900,8 +912,16 @@ static int msix_capability_init(struct pci_dev *dev,
 
         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;
         }
@@ -1083,9 +1103,14 @@ static void _pci_cleanup_msix(struct arch_msix *msix)
         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;
     }
 }
 
--
generated by git-patchbot for /home/xen/git/xen.git#stable-4.11



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.