* [PATCH] msix: add valid.accepts methods to check address
@ 2020-06-01 5:14 P J P
2020-06-01 6:02 ` Philippe Mathieu-Daudé
2020-06-01 6:05 ` Michael S. Tsirkin
0 siblings, 2 replies; 6+ messages in thread
From: P J P @ 2020-06-01 5:14 UTC (permalink / raw)
To: Michael S . Tsirkin
Cc: Prasad J Pandit, QEMU Developers, Alexander Bulekov,
Anatoly Trosinenko, Ren Ding, Hanqing Zhao
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] msix: add valid.accepts methods to check address
2020-06-01 5:14 [PATCH] msix: add valid.accepts methods to check address P J P
@ 2020-06-01 6:02 ` Philippe Mathieu-Daudé
2020-06-01 6:03 ` Philippe Mathieu-Daudé
2020-06-01 18:54 ` P J P
2020-06-01 6:05 ` Michael S. Tsirkin
1 sibling, 2 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-01 6:02 UTC (permalink / raw)
To: P J P, Michael S . Tsirkin
Cc: Prasad J Pandit, QEMU Developers, Alexander Bulekov,
Anatoly Trosinenko, Ren Ding, Hanqing Zhao
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
> },
> };
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] msix: add valid.accepts methods to check address
2020-06-01 6:02 ` Philippe Mathieu-Daudé
@ 2020-06-01 6:03 ` Philippe Mathieu-Daudé
2020-06-01 18:54 ` P J P
1 sibling, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-01 6:03 UTC (permalink / raw)
To: P J P, Michael S . Tsirkin
Cc: Prasad J Pandit, QEMU Developers, Alexander Bulekov,
Anatoly Trosinenko, Ren Ding, Hanqing Zhao
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
> > },
> > };
> >
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] msix: add valid.accepts methods to check address
2020-06-01 5:14 [PATCH] msix: add valid.accepts methods to check address P J P
2020-06-01 6:02 ` Philippe Mathieu-Daudé
@ 2020-06-01 6:05 ` Michael S. Tsirkin
2020-06-01 6:40 ` P J P
1 sibling, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2020-06-01 6:05 UTC (permalink / raw)
To: P J P
Cc: Prasad J Pandit, QEMU Developers, Alexander Bulekov,
Anatoly Trosinenko, Ren Ding, Hanqing Zhao
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] msix: add valid.accepts methods to check address
2020-06-01 6:05 ` Michael S. Tsirkin
@ 2020-06-01 6:40 ` P J P
0 siblings, 0 replies; 6+ messages in thread
From: P J P @ 2020-06-01 6:40 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: QEMU Developers, Alexander Bulekov, Anatoly Trosinenko, Ren Ding,
Philippe Mathieu-Daudé,
Hanqing Zhao
+-- 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] msix: add valid.accepts methods to check address
2020-06-01 6:02 ` Philippe Mathieu-Daudé
2020-06-01 6:03 ` Philippe Mathieu-Daudé
@ 2020-06-01 18:54 ` P J P
1 sibling, 0 replies; 6+ messages in thread
From: P J P @ 2020-06-01 18:54 UTC (permalink / raw)
To: Michael S . Tsirkin
Cc: QEMU Developers, Alexander Bulekov, Anatoly Trosinenko, Ren Ding,
Philippe Mathieu-Daudé,
Hanqing Zhao
[-- Attachment #1: Type: text/plain, Size: 302 bytes --]
+-- 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-01 18:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-01 5:14 [PATCH] msix: add valid.accepts methods to check address P J P
2020-06-01 6:02 ` Philippe Mathieu-Daudé
2020-06-01 6:03 ` Philippe Mathieu-Daudé
2020-06-01 18:54 ` P J P
2020-06-01 6:05 ` Michael S. Tsirkin
2020-06-01 6:40 ` P J P
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.