[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] msix: add valid.accepts methods to check address
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH] msix: add valid.accepts methods to check address |
Date: |
Mon, 1 Jun 2020 08:02:00 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
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
> },
> };
>
>