diff mbox

i386: amd_iommu: fix MMIO register count and access

Message ID 1480677386-16192-1-git-send-email-ppandit@redhat.com
State New
Headers show

Commit Message

Prasad Pandit Dec. 2, 2016, 11:16 a.m. UTC
From: Prasad J Pandit <pjp@fedoraproject.org>

IOMMU MMIO registers are divided in two groups by their offsets.
Low offsets(<0x2000) registers are grouped into 'amdvi_mmio_low'
table and higher offsets(>=0x2000) registers are grouped into
'amdvi_mmio_high' table. No of registers in each table is given
by macro 'AMDVI_MMIO_REGS_LOW' and 'AMDVI_MMIO_REGS_HIGH' resp.
Values of these two macros were swapped, resulting in an OOB
access when reading 'amdvi_mmio_high' table. Correct these two
macros. Also read from 'amdvi_mmio_low' table for lower address.

Reported-by: Azureyang <azureyang@tencent.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
 hw/i386/amd_iommu.c | 2 +-
 hw/i386/amd_iommu.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Prasad Pandit Dec. 13, 2016, 10:12 a.m. UTC | #1
+-- On Fri, 2 Dec 2016, P J P wrote --+
| IOMMU MMIO registers are divided in two groups by their offsets.
| Low offsets(<0x2000) registers are grouped into 'amdvi_mmio_low'
| table and higher offsets(>=0x2000) registers are grouped into
| 'amdvi_mmio_high' table. No of registers in each table is given
| by macro 'AMDVI_MMIO_REGS_LOW' and 'AMDVI_MMIO_REGS_HIGH' resp.
| Values of these two macros were swapped, resulting in an OOB
| access when reading 'amdvi_mmio_high' table. Correct these two
| macros. Also read from 'amdvi_mmio_low' table for lower address.
| 
| Reported-by: Azureyang <azureyang@tencent.com>
| Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
| ---
|  hw/i386/amd_iommu.c | 2 +-
|  hw/i386/amd_iommu.h | 4 ++--
|  2 files changed, 3 insertions(+), 3 deletions(-)
| 
| diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
| index 47b79d9..e0732cc 100644
| --- a/hw/i386/amd_iommu.c
| +++ b/hw/i386/amd_iommu.c
| @@ -562,7 +562,7 @@ static void amdvi_mmio_trace(hwaddr addr, unsigned size)
|          trace_amdvi_mmio_read(amdvi_mmio_high[index], addr, size, addr & ~0x07);
|      } else {
|          index = index >= AMDVI_MMIO_REGS_LOW ? AMDVI_MMIO_REGS_LOW : index;
| -        trace_amdvi_mmio_read(amdvi_mmio_high[index], addr, size, addr & ~0x07);
| +        trace_amdvi_mmio_read(amdvi_mmio_low[index], addr, size, addr & ~0x07);
|      }
|  }
|  
| diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
| index 884926e..0d3dc6a 100644
| --- a/hw/i386/amd_iommu.h
| +++ b/hw/i386/amd_iommu.h
| @@ -49,8 +49,8 @@
|  #define AMDVI_CAPAB_INIT_TYPE         (3 << 16)
|  
|  /* No. of used MMIO registers */
| -#define AMDVI_MMIO_REGS_HIGH  8
| -#define AMDVI_MMIO_REGS_LOW   7
| +#define AMDVI_MMIO_REGS_HIGH  7
| +#define AMDVI_MMIO_REGS_LOW   8
|  
|  /* MMIO registers */
|  #define AMDVI_MMIO_DEVICE_TABLE       0x0000


Ping..!
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
Michael S. Tsirkin Dec. 13, 2016, 4:25 p.m. UTC | #2
On Tue, Dec 13, 2016 at 03:42:13PM +0530, P J P wrote:
> +-- On Fri, 2 Dec 2016, P J P wrote --+
> | IOMMU MMIO registers are divided in two groups by their offsets.
> | Low offsets(<0x2000) registers are grouped into 'amdvi_mmio_low'
> | table and higher offsets(>=0x2000) registers are grouped into
> | 'amdvi_mmio_high' table. No of registers in each table is given
> | by macro 'AMDVI_MMIO_REGS_LOW' and 'AMDVI_MMIO_REGS_HIGH' resp.
> | Values of these two macros were swapped, resulting in an OOB
> | access when reading 'amdvi_mmio_high' table. Correct these two
> | macros. Also read from 'amdvi_mmio_low' table for lower address.
> | 
> | Reported-by: Azureyang <azureyang@tencent.com>
> | Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> | ---
> |  hw/i386/amd_iommu.c | 2 +-
> |  hw/i386/amd_iommu.h | 4 ++--
> |  2 files changed, 3 insertions(+), 3 deletions(-)
> | 
> | diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> | index 47b79d9..e0732cc 100644
> | --- a/hw/i386/amd_iommu.c
> | +++ b/hw/i386/amd_iommu.c
> | @@ -562,7 +562,7 @@ static void amdvi_mmio_trace(hwaddr addr, unsigned size)
> |          trace_amdvi_mmio_read(amdvi_mmio_high[index], addr, size, addr & ~0x07);
> |      } else {
> |          index = index >= AMDVI_MMIO_REGS_LOW ? AMDVI_MMIO_REGS_LOW : index;
> | -        trace_amdvi_mmio_read(amdvi_mmio_high[index], addr, size, addr & ~0x07);
> | +        trace_amdvi_mmio_read(amdvi_mmio_low[index], addr, size, addr & ~0x07);
> |      }
> |  }
> |  
> | diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
> | index 884926e..0d3dc6a 100644
> | --- a/hw/i386/amd_iommu.h
> | +++ b/hw/i386/amd_iommu.h
> | @@ -49,8 +49,8 @@
> |  #define AMDVI_CAPAB_INIT_TYPE         (3 << 16)
> |  
> |  /* No. of used MMIO registers */
> | -#define AMDVI_MMIO_REGS_HIGH  8
> | -#define AMDVI_MMIO_REGS_LOW   7
> | +#define AMDVI_MMIO_REGS_HIGH  7
> | +#define AMDVI_MMIO_REGS_LOW   8
> |  
> |  /* MMIO registers */
> |  #define AMDVI_MMIO_DEVICE_TABLE       0x0000
> 
> 
> Ping..!


Sorry missed 2.8. Wil be in 2.8.1
> --
> Prasad J Pandit / Red Hat Product Security Team
> 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
diff mbox

Patch

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 47b79d9..e0732cc 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -562,7 +562,7 @@  static void amdvi_mmio_trace(hwaddr addr, unsigned size)
         trace_amdvi_mmio_read(amdvi_mmio_high[index], addr, size, addr & ~0x07);
     } else {
         index = index >= AMDVI_MMIO_REGS_LOW ? AMDVI_MMIO_REGS_LOW : index;
-        trace_amdvi_mmio_read(amdvi_mmio_high[index], addr, size, addr & ~0x07);
+        trace_amdvi_mmio_read(amdvi_mmio_low[index], addr, size, addr & ~0x07);
     }
 }
 
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 884926e..0d3dc6a 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -49,8 +49,8 @@ 
 #define AMDVI_CAPAB_INIT_TYPE         (3 << 16)
 
 /* No. of used MMIO registers */
-#define AMDVI_MMIO_REGS_HIGH  8
-#define AMDVI_MMIO_REGS_LOW   7
+#define AMDVI_MMIO_REGS_HIGH  7
+#define AMDVI_MMIO_REGS_LOW   8
 
 /* MMIO registers */
 #define AMDVI_MMIO_DEVICE_TABLE       0x0000