| Message ID | 20200811114133.672647-10-ppandit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | memory: assert and define MemoryRegionOps callbacks | expand |
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; >
+-- 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
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;