From: Prasad J Pandit <pjp@fedoraproject.org>
While doing msi-x mmio operations, a guest may send an address
that leads to an OOB access issue. Add valid.accepts methods to
ensure that ensuing mmio r/w operation don't go beyond regions.
Reported-by: Ren Ding <rding@gatech.edu>
Reported-by: Hanqing Zhao <hanqing@gatech.edu>
Reported-by: Anatoly Trosinenko <anatoly.trosinenko@gmail.com>
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
hw/pci/msix.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 29187898f2..d90d66a3b8 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -193,6 +193,15 @@ static void msix_table_mmio_write(void *opaque, hwaddr addr,
msix_handle_mask_update(dev, vector, was_masked);
}
+static bool msix_table_accepts(void *opaque, hwaddr addr, unsigned size,
+ bool is_write, MemTxAttrs attrs)
+{
+ PCIDevice *dev = opaque;
+ uint16_t tbl_size = dev->msix_entries_nr * PCI_MSIX_ENTRY_SIZE;
+
+ return dev->msix_table + addr + 4 <= dev->msix_table + tbl_size;
+}
+
static const MemoryRegionOps msix_table_mmio_ops = {
.read = msix_table_mmio_read,
.write = msix_table_mmio_write,
@@ -200,6 +209,7 @@ static const MemoryRegionOps msix_table_mmio_ops = {
.valid = {
.min_access_size = 4,
.max_access_size = 4,
+ .accepts = msix_table_accepts
},
};
@@ -221,6 +231,15 @@ static void msix_pba_mmio_write(void *opaque, hwaddr addr,
{
}
+static bool msix_pba_accepts(void *opaque, hwaddr addr, unsigned size,
+ bool is_write, MemTxAttrs attrs)
+{
+ PCIDevice *dev = opaque;
+ uint16_t pba_size = QEMU_ALIGN_UP(dev->msix_entries_nr, 64) / 8;
+
+ return dev->msix_pba + addr + 4 <= dev->msix_pba + pba_size;
+}
+
static const MemoryRegionOps msix_pba_mmio_ops = {
.read = msix_pba_mmio_read,
.write = msix_pba_mmio_write,
@@ -228,6 +247,7 @@ static const MemoryRegionOps msix_pba_mmio_ops = {
.valid = {
.min_access_size = 4,
.max_access_size = 4,
+ .accepts = msix_pba_accepts
},
};
--
2.26.2
On 6/1/20 7:14 AM, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > While doing msi-x mmio operations, a guest may send an address > that leads to an OOB access issue. Add valid.accepts methods to > ensure that ensuing mmio r/w operation don't go beyond regions. > Fixes: CVE-2020-xxxxx > Reported-by: Ren Ding <rding@gatech.edu> > Reported-by: Hanqing Zhao <hanqing@gatech.edu> > Reported-by: Anatoly Trosinenko <anatoly.trosinenko@gmail.com> > Reported-by: Alexander Bulekov <alxndr@bu.edu> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/pci/msix.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/hw/pci/msix.c b/hw/pci/msix.c > index 29187898f2..d90d66a3b8 100644 > --- a/hw/pci/msix.c > +++ b/hw/pci/msix.c > @@ -193,6 +193,15 @@ static void msix_table_mmio_write(void *opaque, hwaddr addr, > msix_handle_mask_update(dev, vector, was_masked); > } > > +static bool msix_table_accepts(void *opaque, hwaddr addr, unsigned size, > + bool is_write, MemTxAttrs attrs) > +{ > + PCIDevice *dev = opaque; > + uint16_t tbl_size = dev->msix_entries_nr * PCI_MSIX_ENTRY_SIZE; > + > + return dev->msix_table + addr + 4 <= dev->msix_table + tbl_size; Can be simplified as: return addr + 4 <= dev->msix_entries_nr * PCI_MSIX_ENTRY_SIZE; > +} > + > static const MemoryRegionOps msix_table_mmio_ops = { > .read = msix_table_mmio_read, > .write = msix_table_mmio_write, > @@ -200,6 +209,7 @@ static const MemoryRegionOps msix_table_mmio_ops = { > .valid = { > .min_access_size = 4, > .max_access_size = 4, > + .accepts = msix_table_accepts > }, > }; > > @@ -221,6 +231,15 @@ static void msix_pba_mmio_write(void *opaque, hwaddr addr, > { > } > > +static bool msix_pba_accepts(void *opaque, hwaddr addr, unsigned size, > + bool is_write, MemTxAttrs attrs) > +{ > + PCIDevice *dev = opaque; > + uint16_t pba_size = QEMU_ALIGN_UP(dev->msix_entries_nr, 64) / 8; > + > + return dev->msix_pba + addr + 4 <= dev->msix_pba + pba_size; Can be simplified as: return addr + 4 <= QEMU_ALIGN_UP(dev->msix_entries_nr, 64) / 8; > +} > + > static const MemoryRegionOps msix_pba_mmio_ops = { > .read = msix_pba_mmio_read, > .write = msix_pba_mmio_write, > @@ -228,6 +247,7 @@ static const MemoryRegionOps msix_pba_mmio_ops = { > .valid = { > .min_access_size = 4, > .max_access_size = 4, > + .accepts = msix_pba_accepts > }, > }; > >
On Mon, Jun 1, 2020 at 8:02 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > On 6/1/20 7:14 AM, P J P wrote: > > From: Prasad J Pandit <pjp@fedoraproject.org> > > > > While doing msi-x mmio operations, a guest may send an address > > that leads to an OOB access issue. Add valid.accepts methods to > > ensure that ensuing mmio r/w operation don't go beyond regions. > > > > Fixes: CVE-2020-xxxxx Oh and please also add: Cc: qemu-stable@nongnu.org > > > Reported-by: Ren Ding <rding@gatech.edu> > > Reported-by: Hanqing Zhao <hanqing@gatech.edu> > > Reported-by: Anatoly Trosinenko <anatoly.trosinenko@gmail.com> > > Reported-by: Alexander Bulekov <alxndr@bu.edu> > > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > > --- > > hw/pci/msix.c | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/hw/pci/msix.c b/hw/pci/msix.c > > index 29187898f2..d90d66a3b8 100644 > > --- a/hw/pci/msix.c > > +++ b/hw/pci/msix.c > > @@ -193,6 +193,15 @@ static void msix_table_mmio_write(void *opaque, hwaddr addr, > > msix_handle_mask_update(dev, vector, was_masked); > > } > > > > +static bool msix_table_accepts(void *opaque, hwaddr addr, unsigned size, > > + bool is_write, MemTxAttrs attrs) > > +{ > > + PCIDevice *dev = opaque; > > + uint16_t tbl_size = dev->msix_entries_nr * PCI_MSIX_ENTRY_SIZE; > > + > > + return dev->msix_table + addr + 4 <= dev->msix_table + tbl_size; > > Can be simplified as: > > return addr + 4 <= dev->msix_entries_nr * PCI_MSIX_ENTRY_SIZE; > > > +} > > + > > static const MemoryRegionOps msix_table_mmio_ops = { > > .read = msix_table_mmio_read, > > .write = msix_table_mmio_write, > > @@ -200,6 +209,7 @@ static const MemoryRegionOps msix_table_mmio_ops = { > > .valid = { > > .min_access_size = 4, > > .max_access_size = 4, > > + .accepts = msix_table_accepts > > }, > > }; > > > > @@ -221,6 +231,15 @@ static void msix_pba_mmio_write(void *opaque, hwaddr addr, > > { > > } > > > > +static bool msix_pba_accepts(void *opaque, hwaddr addr, unsigned size, > > + bool is_write, MemTxAttrs attrs) > > +{ > > + PCIDevice *dev = opaque; > > + uint16_t pba_size = QEMU_ALIGN_UP(dev->msix_entries_nr, 64) / 8; > > + > > + return dev->msix_pba + addr + 4 <= dev->msix_pba + pba_size; > > Can be simplified as: > > return addr + 4 <= QEMU_ALIGN_UP(dev->msix_entries_nr, 64) / 8; > > > +} > > + > > static const MemoryRegionOps msix_pba_mmio_ops = { > > .read = msix_pba_mmio_read, > > .write = msix_pba_mmio_write, > > @@ -228,6 +247,7 @@ static const MemoryRegionOps msix_pba_mmio_ops = { > > .valid = { > > .min_access_size = 4, > > .max_access_size = 4, > > + .accepts = msix_pba_accepts > > }, > > }; > > > > >
+-- On Mon, 1 Jun 2020, Philippe Mathieu-Daudé wrote --+ | Fixes: CVE-2020-xxxxx 'CVE-2020-13754' assigned to this issue by Mitre. -> https://bugzilla.redhat.com/show_bug.cgi?id=1842363 Thank you. -- Prasad J Pandit / Red Hat Product Security Team 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
On Mon, Jun 01, 2020 at 10:44:54AM +0530, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > While doing msi-x mmio operations, a guest may send an address > that leads to an OOB access issue. Add valid.accepts methods to > ensure that ensuing mmio r/w operation don't go beyond regions. > > Reported-by: Ren Ding <rding@gatech.edu> > Reported-by: Hanqing Zhao <hanqing@gatech.edu> > Reported-by: Anatoly Trosinenko <anatoly.trosinenko@gmail.com> > Reported-by: Alexander Bulekov <alxndr@bu.edu> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> IMHO this is just messed up, memory core needs to guarantee this. I'm working on a patch to do that. > --- > hw/pci/msix.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/hw/pci/msix.c b/hw/pci/msix.c > index 29187898f2..d90d66a3b8 100644 > --- a/hw/pci/msix.c > +++ b/hw/pci/msix.c > @@ -193,6 +193,15 @@ static void msix_table_mmio_write(void *opaque, hwaddr addr, > msix_handle_mask_update(dev, vector, was_masked); > } > > +static bool msix_table_accepts(void *opaque, hwaddr addr, unsigned size, > + bool is_write, MemTxAttrs attrs) > +{ > + PCIDevice *dev = opaque; > + uint16_t tbl_size = dev->msix_entries_nr * PCI_MSIX_ENTRY_SIZE; > + > + return dev->msix_table + addr + 4 <= dev->msix_table + tbl_size; > +} > + > static const MemoryRegionOps msix_table_mmio_ops = { > .read = msix_table_mmio_read, > .write = msix_table_mmio_write, > @@ -200,6 +209,7 @@ static const MemoryRegionOps msix_table_mmio_ops = { > .valid = { > .min_access_size = 4, > .max_access_size = 4, > + .accepts = msix_table_accepts > }, > }; > > @@ -221,6 +231,15 @@ static void msix_pba_mmio_write(void *opaque, hwaddr addr, > { > } > > +static bool msix_pba_accepts(void *opaque, hwaddr addr, unsigned size, > + bool is_write, MemTxAttrs attrs) > +{ > + PCIDevice *dev = opaque; > + uint16_t pba_size = QEMU_ALIGN_UP(dev->msix_entries_nr, 64) / 8; > + > + return dev->msix_pba + addr + 4 <= dev->msix_pba + pba_size; > +} > + > static const MemoryRegionOps msix_pba_mmio_ops = { > .read = msix_pba_mmio_read, > .write = msix_pba_mmio_write, > @@ -228,6 +247,7 @@ static const MemoryRegionOps msix_pba_mmio_ops = { > .valid = { > .min_access_size = 4, > .max_access_size = 4, > + .accepts = msix_pba_accepts > }, > }; > > -- > 2.26.2
+-- On Mon, 1 Jun 2020, Michael S. Tsirkin wrote --+ | IMHO this is just messed up, memory core needs to guarantee this. | I'm working on a patch to do that. Okay. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
© 2016 - 2024 Red Hat, Inc.