[Xen-devel] [PATCH] x86/msi: fix loop termination condition in pci_msi_conf_write_intercept()

[Paul Durrant <paul.durrant@xxxxxxxxxx>]

The for loop that deals with MSI masking is coded as follows:

for ( pos = 0; pos < entry->msi.nvec; ++pos, ++entry )

Thus the loop termination condition is dereferencing a struct pointer that
is being incremented by the loop. However, it is clear from following code
paths in msi_capability_init() that this is unsafe as for instance, in the
case of nvec == 1, entry will point at a single struct msi_desc allocation
and thus the loop will walk beyond the bounds of the allocation before
dereferencing the memory to determine whether the loop should terminate.
Also, because the body of the loop writes via the entry pointer, this can
then lead to heap memory corruption, or indeed corruption of anything in
the direct map.

This patch simply initializes a stack variable to the value of
entry->msi.nvec before starting the loop and then uses that in the
termination condition instead.

Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
---
Cc: Jan Beulich <jbeulich@xxxxxxxx>
Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Cc: Wei Liu <wl@xxxxxxx>
Cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
Cc: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>

Credit to Andrew Cooper and Igor Druzhinin for helping narrow down the
source of the memory corruption. It has taken many weeks of head-scratching
to get to this fix.
---
 xen/arch/x86/msi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index babc4147c4..89e61160e9 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -1328,6 +1328,7 @@ int pci_msi_conf_write_intercept(struct pci_dev *pdev, 
unsigned int reg,
     {
         uint16_t cntl;
         uint32_t unused;
+        unsigned int nvec = entry->msi.nvec;
 
         pos = entry->msi_attrib.pos;
         if ( reg < pos || reg >= entry->msi.mpos + 8 )
@@ -1340,7 +1341,7 @@ int pci_msi_conf_write_intercept(struct pci_dev *pdev, 
unsigned int reg,
 
         cntl = pci_conf_read16(seg, bus, slot, func, msi_control_reg(pos));
         unused = ~(uint32_t)0 >> (32 - multi_msi_capable(cntl));
-        for ( pos = 0; pos < entry->msi.nvec; ++pos, ++entry )
+        for ( pos = 0; pos < nvec; ++pos, ++entry )
         {
             entry->msi_attrib.guest_masked =
                 *data >> entry->msi_attrib.entry_nr;
-- 
2.20.1.2.gb21ebb671


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel