Skip to contentSkip to conversation

Xen Security Advisory 524 v1 (CVE-2024-21963) - AMD IOMMU Out-of-Bounds Device Table Read

PATCH Xen.org security team 1 participant 1 message Nov 10

lore
Share & tools
lore
#0 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Xen Security Advisory CVE-2024-21963 / XSA-524

AMD IOMMU Out-of-Bounds Device Table Read

ISSUE DESCRIPTION
=================

With AMD IOMMUs, the OS or Hypervisor chooses the position and size of the Device Table, which holds the main configuration information for devices.

AMD have disclosed a vulnerability in some IOMMUs, whereby under specific circumstances they read beyond the size chosen by the OS or Hypervisor. This gets interpreted as valid configuration data for certain devices.

For more information, see:
https://www.amd.com/en/resources/product-security/bulletin/amd-sb-3060.html

IMPACT
======

Depending on platform and system configuration, a domain might be able to control the IOMMU configuration for a device.

If a domain can exercise such control, it can modify arbitrary host memory and escalate it's privilege to that of Xen.

VULNERABLE SYSTEMS
==================

Systems running all versions of Xen are affected.

Only certain AMD systems are believed to be vulnerable. Systems from other manufacturers are not known to be affected.

A particular system is potentially vulnerable if:

1) Any of the IOMMUs in the system have a Device Table size less than 2MB. i.e. the system's IVRS table describes that particular IOMMU as not covering a full PCI bus, and
2) There is either:
a) A real device behind the IOMMU which is the first function not covered by the IVRS description, or
b) Any device behind the IOMMU which can be configured to use the initiator ID of the first function not covered by the IVRS description.

If the above situation holds, the IOMMU will read out of bounds from the next sequential page in physical memory, and interpret it as valid configuration data for the device. This page may belong to a guest.

MITIGATION
==========

There are no mitigations.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

Note that patches for released versions are generally prepared to apply to the stable branches, and may not apply cleanly to the most recent release tarball. Downstreams are encouraged to update to the tip of the stable branch before applying these patches.

xsa524.patch xen-unstable - Xen 4.17.x

$ sha256sum xsa524*
01204a1bacc2bfb44506975a147fbf5f067831eebee37b7f8cf55f558adceb50  xsa524.patch
$

NOTE CONCERNING CVE
===================

The use of a 2024 CVE on a 2026 disclosure was queried with AMD, and confirmed to be correct.
-----BEGIN PGP SIGNATURE-----

iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmrzWUUMHHBncEB4ZW4u b3JnAAoJEIP+FMlX6CvZF3oIALkMaorBfSljZ7A0pDIjp9Ejy3vkQgMfMnyDpmnh n/M0rdHDaX+hpavhsCThZd65j6fwLiLqeKC14FV9dxmb3VqdW7Efpc6J/LZ3Z3ZW hbtej/hGRfjWGKF4El6FH4Lzx5AjDmKqDyvR71+TL2sXT80xvKmWfVhx8MGG+JGb lKuBp0lS+72qhIGdMr3IWOebYi0h75JA0XgPlm2YGajn7CL0RpImO78gb5WvT5sU /Bv9kUBhsZsitTEV8vXdF4A0ksZM9xIYQavsfq+4YyRGYmAYwQqR3mYHF2SJdq4R MGsY5LxhFcktw+dfRq9rtfU/vnVkfxDBgzQrwRFts8YbCqI=
=bZvY
-----END PGP SIGNATURE-----

From: Jan Beulich <[email protected]>
Subject: AMD/IOMMU: allocate (at least) one spare device table entry

Certain IOMMUs have an off-by-one in the upper bounds check of device table accesses. Therefore an extra entry is needed, to avoid the IOMMU accessing random unrelated memory.

Since the size is reported to the IOMMU in 4k granularity, an extra page may need allocating. To keep things simple (as before: full order-N allocations), arrange to allocate the next higher order of pages if/as necessary. At the same time avoid using / reporting the extra entries.

As struct table_struct's alloc_size field is now only ever written, drop the field right away (if for nothing else than to avoid possible Misra concerns).

This is CVE-2024-21963 / XSA-524.

xen/drivers/passthrough/amd/iommu-defs.h +1 -1
index 30b094f18768..72c757918531 100644
@@ -104,7 +104,7 @@ struct amd_iommu_dte {
     /* 0 - 63 */
     bool v:1;
     bool tv:1;
-    unsigned int :5;
+    unsigned int rsvd:5;
     unsigned int had:2;
 #define IOMMU_MAX_PT_LEVELS 6
     unsigned int paging_mode:3;
xen/drivers/passthrough/amd/iommu.h +0 -1
index 52f748310be4..042c6eed59b2 100644
@@ -54,7 +54,6 @@ typedef struct cmd_entry
 struct table_struct {
     void *buffer;
     unsigned long entries;
-    unsigned long alloc_size;
 };
 
 struct ring_buffer {
xen/drivers/passthrough/amd/iommu_init.c +31 -7
index e0c8925c33f7..9376ca612099 100644
@@ -80,7 +80,9 @@ static void register_iommu_dev_table_in_mmio_space(struct amd_iommu *iommu)
 
     entry = 0;
     iommu_set_addr_lo_to_reg(&entry, addr_lo >> PAGE_SHIFT);
-    set_field_in_reg_u32((iommu->dev_table.alloc_size / PAGE_SIZE) - 1,
+    set_field_in_reg_u32(ROUNDUP(iommu->dev_table.entries *
+                                 IOMMU_DEV_TABLE_ENTRY_SIZE,
+                                 PAGE_SIZE) - 1,
                          entry, IOMMU_DEV_TABLE_SIZE_MASK,
                          IOMMU_DEV_TABLE_SIZE_SHIFT, &entry);
     writel(entry, iommu->mmio_base + IOMMU_DEV_TABLE_BASE_LOW_OFFSET);
@@ -948,8 +950,21 @@ static void disable_iommu(struct amd_iommu *iommu)
 
 static unsigned int __init dt_alloc_size(void)
 {
-    return PAGE_SIZE << get_order_from_bytes(ivrs_bdf_entries *
-                                             IOMMU_DEV_TABLE_ENTRY_SIZE);
+    unsigned int nr = ROUNDUP(ivrs_bdf_entries,
+                              PAGE_SIZE / IOMMU_DEV_TABLE_ENTRY_SIZE);
+
+    /*
+     * AMD SN-3060 / CVE-2024-21963
+     *
+     * Some IOMMUs read beyond the programmed Device Table limit.  When the
+     * total size of the Device Table is less than 2M (covering all 2^16 BDFs
+     * in a single PCI Segment), we must allocate one extra entry to avoid the
+     * IOMMU reading unrelated memory and treating it as a valid entry.
+     */
+    if ( nr < (1U << 16) )
+        ++nr;
+
+    return PAGE_SIZE << get_order_from_bytes(nr * IOMMU_DEV_TABLE_ENTRY_SIZE);
 }
 
 static void __init deallocate_buffer(void *buf, unsigned long sz)
@@ -1089,9 +1104,8 @@ static int __init amd_iommu_init_one(struct amd_iommu *iommu, bool intr)
     if ( !IVRS_MAPPINGS_DEVTAB(ivrs_mappings) )
         goto error_out;
 
-    iommu->dev_table.alloc_size = dt_alloc_size();
-    iommu->dev_table.entries = iommu->dev_table.alloc_size /
-                               IOMMU_DEV_TABLE_ENTRY_SIZE;
+    /* There may be trailing "scratch" entries, which should not be used. */
+    iommu->dev_table.entries = ivrs_bdf_entries;
     iommu->dev_table.buffer = IVRS_MAPPINGS_DEVTAB(ivrs_mappings);
 
     enable_iommu(iommu);
@@ -1255,11 +1269,21 @@ static int __init cf_check amd_iommu_setup_device_table(
          * IOMMU_DEV_TABLE_INT_CONTROL_ABORTED all being zero, and us also
          * wanting at least TV, GV, I, and EX set to false.
          */
-        for ( bdf = 0, size /= sizeof(*dt); bdf < size; ++bdf )
+        for ( bdf = 0; bdf < ivrs_bdf_entries; ++bdf )
             dt[bdf] = (struct amd_iommu_dte){
                           .v = true,
                           .iv = iommu_intremap,
                       };
+
+        /*
+         * Prefill extra DTEs such that their use will raise an
+         * ILLEGAL_DEV_TABLE_ENTRY event.
+         */
+        for ( size /= sizeof(*dt); bdf < size; ++bdf )
+            dt[bdf] = (struct amd_iommu_dte){
+                          .v = true,
+                          .rsvd = 3, /* As per AMD recommendation. */
+                      };
     }
 
     /* Add device table entries */