mbox series

[v4,0/9] memory: assert and define MemoryRegionOps callbacks

Message ID 20200811114133.672647-1-ppandit@redhat.com
Headers show
Series memory: assert and define MemoryRegionOps callbacks | expand

Message

Prasad Pandit Aug. 11, 2020, 11:41 a.m. UTC 
From: Prasad J Pandit <pjp@fedoraproject.org>

Hello,

* This series asserts that MemoryRegionOps objects define read/write
  callback methods. Thus avoids potential NULL pointer dereference.
  ex. -> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2

* Also adds various undefined MemoryRegionOps read/write functions
  to avoid potential assert failure.

Thank you.
--
Prasad J Pandit (9):
  hw/pci-host: add pci-intack write method
  pci-host: designware: add pcie-msi read method
  vfio: add quirk device write method
  prep: add ppc-parity write method
  nvram: add nrf51_soc flash read method
  spapr_pci: add spapr msi read method
  tz-ppc: add dummy read/write methods
  imx7-ccm: add digprog mmio write method
  memory: assert MemoryRegionOps callbacks are defined

 hw/misc/imx7_ccm.c       |  8 ++++++++
 hw/misc/tz-ppc.c         | 14 ++++++++++++++
 hw/nvram/nrf51_nvm.c     | 10 ++++++++++
 hw/pci-host/designware.c | 19 +++++++++++++++++++
 hw/pci-host/prep.c       |  8 ++++++++
 hw/ppc/prep_systemio.c   |  8 ++++++++
 hw/ppc/spapr_pci.c       | 14 ++++++++++++--
 hw/vfio/pci-quirks.c     |  8 ++++++++
 softmmu/memory.c         | 10 +++++++++-
 9 files changed, 96 insertions(+), 3 deletions(-)

--
2.26.2

Comments

Prasad Pandit Sept. 15, 2020, 1:33 p.m. UTC | #1 
+-- On Tue, 11 Aug 2020, P J P wrote --+
| * This series asserts that MemoryRegionOps objects define read/write
|   callback methods. Thus avoids potential NULL pointer dereference.
|   ex. -> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2
| 
| * Also adds various undefined MemoryRegionOps read/write functions
|   to avoid potential assert failure.
| 
| Thank you.
| --
| Prasad J Pandit (9):
|   hw/pci-host: add pci-intack write method
|   pci-host: designware: add pcie-msi read method
|   vfio: add quirk device write method
|   prep: add ppc-parity write method
|   nvram: add nrf51_soc flash read method
|   spapr_pci: add spapr msi read method
|   tz-ppc: add dummy read/write methods
|   imx7-ccm: add digprog mmio write method
|   memory: assert MemoryRegionOps callbacks are defined
| 
|  hw/misc/imx7_ccm.c       |  8 ++++++++
|  hw/misc/tz-ppc.c         | 14 ++++++++++++++
|  hw/nvram/nrf51_nvm.c     | 10 ++++++++++
|  hw/pci-host/designware.c | 19 +++++++++++++++++++
|  hw/pci-host/prep.c       |  8 ++++++++
|  hw/ppc/prep_systemio.c   |  8 ++++++++
|  hw/ppc/spapr_pci.c       | 14 ++++++++++++--
|  hw/vfio/pci-quirks.c     |  8 ++++++++
|  softmmu/memory.c         | 10 +++++++++-
|  9 files changed, 96 insertions(+), 3 deletions(-)

Ping...@Peter, @David, @Paolo, @Herve, @Alex,

* This patch series is not pulled/merged yet, could you please help with it?  
  (Pinging respective maintainers, as didn't get reply for last pings, not
   sure how to go about it.)


Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
Philippe Mathieu-Daudé Sept. 16, 2020, 2:17 p.m. UTC | #2 
On 8/11/20 1:41 PM, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
> 
> When registering a MemoryRegionOps object, assert that its
> read/write callback methods are defined. This avoids potential
> guest crash via a NULL pointer dereference.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Li Qiang <liq3ea@gmail.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  softmmu/memory.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> Update v4: add Reviewed-by tag
>   -> https://lists.nongnu.org/archive/html/qemu-devel/2020-07/msg05324.html
> 
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index af25987518..1f4b37b3a6 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -1485,7 +1485,13 @@ void memory_region_init_io(MemoryRegion *mr,
>                             uint64_t size)
>  {
>      memory_region_init(mr, owner, name, size);
> -    mr->ops = ops ? ops : &unassigned_mem_ops;
> +    if (ops) {
> +        assert(ops->read || ops->read_with_attrs);
> +        assert(ops->write || ops->write_with_attrs);
> +        mr->ops = ops;
> +    } else {
> +        mr->ops = &unassigned_mem_ops;
> +    }
>      mr->opaque = opaque;
>      mr->terminates = true;
>  }
> @@ -1663,6 +1669,8 @@ void memory_region_init_rom_device_nomigrate(MemoryRegion *mr,
>  {
>      Error *err = NULL;
>      assert(ops);
> +    assert(ops->read || ops->read_with_attrs);
> +    assert(ops->write || ops->write_with_attrs);
>      memory_region_init(mr, owner, name, size);
>      mr->ops = ops;
>      mr->opaque = opaque;
>
Prasad Pandit Sept. 30, 2020, 6:05 a.m. UTC | #3 
+-- On Wed, 16 Sep 2020, Philippe Mathieu-Daudé wrote --+
| On 8/11/20 1:41 PM, P J P wrote:
| > When registering a MemoryRegionOps object, assert that its
| > read/write callback methods are defined. This avoids potential
| > guest crash via a NULL pointer dereference.
| > 
| > Suggested-by: Peter Maydell <peter.maydell@linaro.org>
| > Reviewed-by: Li Qiang <liq3ea@gmail.com>
| > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
| > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
| 
| Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
| 
| > ---
| >  softmmu/memory.c | 10 +++++++++-
| >  1 file changed, 9 insertions(+), 1 deletion(-)
| > 
| > Update v4: add Reviewed-by tag
| >   -> https://lists.nongnu.org/archive/html/qemu-devel/2020-07/msg05324.html
| > 
| > diff --git a/softmmu/memory.c b/softmmu/memory.c
| > index af25987518..1f4b37b3a6 100644
| > --- a/softmmu/memory.c
| > +++ b/softmmu/memory.c
| > @@ -1485,7 +1485,13 @@ void memory_region_init_io(MemoryRegion *mr,
| >                             uint64_t size)
| >  {
| >      memory_region_init(mr, owner, name, size);
| > -    mr->ops = ops ? ops : &unassigned_mem_ops;
| > +    if (ops) {
| > +        assert(ops->read || ops->read_with_attrs);
| > +        assert(ops->write || ops->write_with_attrs);
| > +        mr->ops = ops;
| > +    } else {
| > +        mr->ops = &unassigned_mem_ops;
| > +    }
| >      mr->opaque = opaque;
| >      mr->terminates = true;
| >  }
| > @@ -1663,6 +1669,8 @@ void memory_region_init_rom_device_nomigrate(MemoryRegion *mr,
| >  {
| >      Error *err = NULL;
| >      assert(ops);
| > +    assert(ops->read || ops->read_with_attrs);
| > +    assert(ops->write || ops->write_with_attrs);
| >      memory_region_init(mr, owner, name, size);
| >      mr->ops = ops;
| >      mr->opaque = opaque;
| > 

@Paolo...ping!

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
Prasad Pandit Nov. 25, 2020, 1:50 p.m. UTC | #4 
+-- On Tue, 15 Sep 2020, P J P wrote --+
| +-- On Tue, 11 Aug 2020, P J P wrote --+
| | * This series asserts that MemoryRegionOps objects define read/write
| |   callback methods. Thus avoids potential NULL pointer dereference.
| |   ex. -> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2
| | 
| | * Also adds various undefined MemoryRegionOps read/write functions
| |   to avoid potential assert failure.
| | 
| | Thank you.
| | --
| | Prasad J Pandit (9):
| |   hw/pci-host: add pci-intack write method
| |   pci-host: designware: add pcie-msi read method
| |   vfio: add quirk device write method
| |   prep: add ppc-parity write method
| |   nvram: add nrf51_soc flash read method
| |   spapr_pci: add spapr msi read method
| |   tz-ppc: add dummy read/write methods
| |   imx7-ccm: add digprog mmio write method
| |   memory: assert MemoryRegionOps callbacks are defined
| | 
| |  hw/misc/imx7_ccm.c       |  8 ++++++++
| |  hw/misc/tz-ppc.c         | 14 ++++++++++++++
| |  hw/nvram/nrf51_nvm.c     | 10 ++++++++++
| |  hw/pci-host/designware.c | 19 +++++++++++++++++++
| |  hw/pci-host/prep.c       |  8 ++++++++
| |  hw/ppc/prep_systemio.c   |  8 ++++++++
| |  hw/ppc/spapr_pci.c       | 14 ++++++++++++--
| |  hw/vfio/pci-quirks.c     |  8 ++++++++
| |  softmmu/memory.c         | 10 +++++++++-
| |  9 files changed, 96 insertions(+), 3 deletions(-)
| 
| Ping...@Peter, @David, @Paolo, @Herve, @Alex,
| 
| * This patch series is not pulled/merged yet, could you please help with it?  


Ping..!
--
Prasad J Pandit / Red Hat Product Security Team
8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D
Paolo Bonzini Feb. 2, 2021, 4:40 p.m. UTC | #5 
On 25/11/20 14:50, P J P wrote:
> +-- On Tue, 15 Sep 2020, P J P wrote --+

> | +-- On Tue, 11 Aug 2020, P J P wrote --+

> | | * This series asserts that MemoryRegionOps objects define read/write

> | |   callback methods. Thus avoids potential NULL pointer dereference.

> | |   ex. -> https://git.qemu.org/?p=qemu.git;a=commit;h=bb15013ef34617eb1344f5276292cadd326c21b2

> | |

> | | * Also adds various undefined MemoryRegionOps read/write functions

> | |   to avoid potential assert failure.

> | |

> | | Thank you.

> | | --

> | | Prasad J Pandit (9):

> | |   hw/pci-host: add pci-intack write method

> | |   pci-host: designware: add pcie-msi read method

> | |   vfio: add quirk device write method

> | |   prep: add ppc-parity write method

> | |   nvram: add nrf51_soc flash read method

> | |   spapr_pci: add spapr msi read method

> | |   tz-ppc: add dummy read/write methods

> | |   imx7-ccm: add digprog mmio write method

> | |   memory: assert MemoryRegionOps callbacks are defined

> | |

> | |  hw/misc/imx7_ccm.c       |  8 ++++++++

> | |  hw/misc/tz-ppc.c         | 14 ++++++++++++++

> | |  hw/nvram/nrf51_nvm.c     | 10 ++++++++++

> | |  hw/pci-host/designware.c | 19 +++++++++++++++++++

> | |  hw/pci-host/prep.c       |  8 ++++++++

> | |  hw/ppc/prep_systemio.c   |  8 ++++++++

> | |  hw/ppc/spapr_pci.c       | 14 ++++++++++++--

> | |  hw/vfio/pci-quirks.c     |  8 ++++++++

> | |  softmmu/memory.c         | 10 +++++++++-

> | |  9 files changed, 96 insertions(+), 3 deletions(-)

> |

> | Ping...@Peter, @David, @Paolo, @Herve, @Alex,

> |

> | * This patch series is not pulled/merged yet, could you please help with it?

> 

> 

> Ping..!

> --

> Prasad J Pandit / Red Hat Product Security Team

> 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D

> 


Queued, thanks!

Paolo