xen/pt: unknown PCI config space fields should be read-only

... by default. Add a per-device "permissive" mode similar to pciback's
to allow restoring previous behavior (and hence break security again,
i.e. should be used only for trusted guests).

This is part of XSA-131.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>)

diff --git a/hw/xen_pt.c b/hw/xen_pt.c
index 8056bbe1eb187055f0492b29b8aa8c3a0507d7cd..66e3c3d379a6ddb6abd5d64717748d6d489cfc19 100644 (file)
--- a/hw/xen_pt.c
+++ b/hw/xen_pt.c
@@ -239,6 +239,7 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
     XenPTReg *reg_entry = NULL;
     uint32_t find_addr = addr;
     XenPTRegInfo *reg = NULL;
+    bool wp_flag = false;
 
     if (xen_pt_pci_config_access_check(d, addr, len)) {
         return;
@@ -278,6 +279,10 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
 
     /* pass directly to the real device for passthrough type register group */
     if (reg_grp_entry == NULL) {
+        if (!s->permissive) {
+            wb_mask = 0;
+            wp_flag = true;
+        }
         goto out;
     }
 
@@ -298,12 +303,15 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
             uint32_t real_offset = reg_grp_entry->base_offset + reg->offset;
             uint32_t valid_mask = 0xFFFFFFFF >> ((4 - emul_len) << 3);
             uint8_t *ptr_val = NULL;
+            uint32_t wp_mask = reg->emu_mask | reg->ro_mask;
 
             valid_mask <<= (find_addr - real_offset) << 3;
             ptr_val = (uint8_t *)&val + (real_offset & 3);
-            if (reg->emu_mask == (0xFFFFFFFF >> ((4 - reg->size) << 3))) {
-                wb_mask &= ~((reg->emu_mask
-                              >> ((find_addr - real_offset) << 3))
+            if (!s->permissive) {
+                wp_mask |= reg->res_mask;
+            }
+            if (wp_mask == (0xFFFFFFFF >> ((4 - reg->size) << 3))) {
+                wb_mask &= ~((wp_mask >> ((find_addr - real_offset) << 3))
                              << ((len - emul_len) << 3));
             }
 
@@ -347,6 +355,16 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
         } else {
             /* nothing to do with passthrough type register,
              * continue to find next byte */
+            if (!s->permissive) {
+                wb_mask &= ~(0xff << ((len - emul_len) << 3));
+                /* Unused BARs will make it here, but we don't want to issue
+                 * warnings for writes to them (bogus writes get dealt with
+                 * above).
+                 */
+                if (index < 0) {
+                    wp_flag = true;
+                }
+            }
             emul_len--;
             find_addr++;
         }
@@ -358,6 +376,13 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
     memory_region_transaction_commit();
 
 out:
+    if (wp_flag && !s->permissive_warned) {
+        s->permissive_warned = true;
+        xen_pt_log(d, "Write-back to unknown field 0x%02x (partially) inhibited (0x%0*x)\n",
+                   addr, len * 2, wb_mask);
+        xen_pt_log(d, "If the device doesn't work, try enabling permissive mode\n");
+        xen_pt_log(d, "(unsafe) and if it helps report the problem to xen-devel\n");
+    }
     for (index = 0; wb_mask; index += len) {
         /* unknown regs are passed through */
         while (!(wb_mask & 0xff)) {
@@ -838,6 +863,7 @@ static void xen_pt_unregister_device(PCIDevice *d)
 
 static Property xen_pci_passthrough_properties[] = {
     DEFINE_PROP_PCI_HOST_DEVADDR("hostaddr", XenPCIPassthroughState, hostaddr),
+    DEFINE_PROP_UINT8("permissive", XenPCIPassthroughState, permissive, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 

diff --git a/hw/xen_pt.h b/hw/xen_pt.h
index 2b6ef1de0e55c1926e418764d7e186d9849cc5a9..c3ea3e1f9a8371a651e161e6870b45a4d001c2ef 100644 (file)
--- a/hw/xen_pt.h
+++ b/hw/xen_pt.h
@@ -197,6 +197,8 @@ struct XenPCIPassthroughState {
 
     PCIHostDeviceAddress hostaddr;
     bool is_virtfn;
+    uint8_t permissive;
+    bool permissive_warned;
     XenHostPCIDevice real_device;
     XenPTRegion bases[PCI_NUM_REGIONS]; /* Access regions */
     QLIST_HEAD(, XenPTRegGroup) reg_grps;

diff --git a/hw/xen_pt_config_init.c b/hw/xen_pt_config_init.c
index 2bbc5059a1a0afd10d03f44f40c092dfe931de2b..442431a962040890671b5fc149233fe490fdd515 100644 (file)
--- a/hw/xen_pt_config_init.c
+++ b/hw/xen_pt_config_init.c
@@ -101,6 +101,10 @@ static uint32_t get_throughable_mask(const XenPCIPassthroughState *s,
 {
     uint32_t throughable_mask = ~(reg->emu_mask | reg->ro_mask);
 
+    if (!s->permissive) {
+        throughable_mask &= ~reg->res_mask;
+    }
+
     return throughable_mask & valid_mask;
 }