P J P <ppan...@redhat.com> writes:
> From: Prasad J Pandit <p...@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.mayd...@linaro.org> > Signed-off-by: Prasad J Pandit <p...@fedoraproject.org> > --- > memory.c | 5 +++++ > 1 file changed, 5 insertions(+) > > Update v1: add assert while registering MemoryRegionOps > -> https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg05187.html > > diff --git a/memory.c b/memory.c > index 91ceaf9fcf..6e94fd5958 100644 > --- a/memory.c > +++ b/memory.c > @@ -1495,6 +1495,9 @@ void memory_region_init_io(MemoryRegion *mr, > const char *name, > uint64_t size) > { > + assert(ops); > + assert(ops->read); > + assert(ops->write); If you look at memory_region_dispatch_write you can see that mr->ops->write being empty is acceptable because it implies mr->ops->write_with_attrs is set instead. I think the same is true for read so I think you need something more like: assert(ops->read || ops->read_with_attrs); assert(ops->write || ops->write_with_attrs); > memory_region_init(mr, owner, name, size); > mr->ops = ops ? ops : &unassigned_mem_ops; > mr->opaque = opaque; > @@ -1674,6 +1677,8 @@ void > memory_region_init_rom_device_nomigrate(MemoryRegion *mr, > { > Error *err = NULL; > assert(ops); > + assert(ops->read); > + assert(ops->write); Do ROM devices need a ->write function? Also doesn't this break a load of running stuff without fixes for all the various missing bits? How far does make check-acceptance get? > memory_region_init(mr, owner, name, size); > mr->ops = ops; > mr->opaque = opaque; -- Alex Bennée
