qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] memory: Add function pointers checks to memory_region_read/write()
@ 2015-07-28  7:13 Salva Peiró
  2015-07-28  7:13 ` [Qemu-devel] [PATCH] " Salva Peiró
  0 siblings, 1 reply; 6+ messages in thread
From: Salva Peiró @ 2015-07-28  7:13 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel


The situation where QEMU crashes while attempting to call to a NULL
function pointer from a non-initialised field in the MemoryRegionOps
struct happens for the majority of emulated devices:
One approach for solving this is to correct it for each device. 
The other approach is to correct the memory_region_read/write caller
functions at memory.c to ensure that only initialised function pointers
are being called. This approach has the benefit of solving this kind of
error for all emulated devices.

The following patch adds function pointers checks to memory_region_read/write()

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH] memory: Add function pointers checks to memory_region_read/write()
@ 2015-09-03 17:37 Salva Peiró
  2015-09-07 10:27 ` Paolo Bonzini
  0 siblings, 1 reply; 6+ messages in thread
From: Salva Peiró @ 2015-09-03 17:37 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, Salva Peiró

The file memory.c directly calls the function pointers provided in
the MemoryRegionOps to handle read and write operations for memory regions.
The function pointers are called without checking if the function
pointers are initialised, therefore, causing QEMU to SIGSEGV when
accessing a memory address for which the operation is not defined (and not initialised)

The patch adds explicit checks to function pointers before issuing the calls.

Signed-off-by: Salva Peiró <speirofr@gmail.com>
---
 memory.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/memory.c b/memory.c
index 5e5f325..d588c5f 100644
--- a/memory.c
+++ b/memory.c
@@ -380,6 +380,9 @@ static MemTxResult memory_region_oldmmio_read_accessor(MemoryRegion *mr,
 {
     uint64_t tmp;
 
+    if (!mr->ops->old_mmio.read) {
+        return MEMTX_ERROR;
+    }
     tmp = mr->ops->old_mmio.read[ctz32(size)](mr->opaque, addr);
     trace_memory_region_ops_read(mr, addr, tmp, size);
     *value |= (tmp & mask) << shift;
@@ -396,6 +399,9 @@ static MemTxResult  memory_region_read_accessor(MemoryRegion *mr,
 {
     uint64_t tmp;
 
+    if (!mr->ops->read) {
+        return MEMTX_ERROR;
+    }
     tmp = mr->ops->read(mr->opaque, addr, size);
     trace_memory_region_ops_read(mr, addr, tmp, size);
     *value |= (tmp & mask) << shift;
@@ -413,6 +419,9 @@ static MemTxResult memory_region_read_with_attrs_accessor(MemoryRegion *mr,
     uint64_t tmp = 0;
     MemTxResult r;
 
+    if (!mr->ops->read_with_attrs) {
+        return MEMTX_ERROR;
+    }
     r = mr->ops->read_with_attrs(mr->opaque, addr, &tmp, size, attrs);
     trace_memory_region_ops_read(mr, addr, tmp, size);
     *value |= (tmp & mask) << shift;
@@ -429,6 +438,9 @@ static MemTxResult memory_region_oldmmio_write_accessor(MemoryRegion *mr,
 {
     uint64_t tmp;
 
+    if (!mr->ops->old_mmio.write) {
+        return MEMTX_ERROR;
+    }
     tmp = (*value >> shift) & mask;
     trace_memory_region_ops_write(mr, addr, tmp, size);
     mr->ops->old_mmio.write[ctz32(size)](mr->opaque, addr, tmp);
@@ -447,6 +459,9 @@ static MemTxResult memory_region_write_accessor(MemoryRegion *mr,
 
     tmp = (*value >> shift) & mask;
     trace_memory_region_ops_write(mr, addr, tmp, size);
+    if (!mr->ops->write) {
+        return MEMTX_ERROR;
+    }
     mr->ops->write(mr->opaque, addr, tmp, size);
     return MEMTX_OK;
 }
@@ -463,6 +478,9 @@ static MemTxResult memory_region_write_with_attrs_accessor(MemoryRegion *mr,
 
     tmp = (*value >> shift) & mask;
     trace_memory_region_ops_write(mr, addr, tmp, size);
+    if (!mr->ops->write_with_attrs) {
+        return MEMTX_ERROR;
+    }
     return mr->ops->write_with_attrs(mr->opaque, addr, tmp, size, attrs);
 }
 
@@ -1187,6 +1205,8 @@ void memory_region_init_io(MemoryRegion *mr,
                            uint64_t size)
 {
     memory_region_init(mr, owner, name, size);
+    assert(ops != NULL);
+
     mr->ops = ops;
     mr->opaque = opaque;
     mr->terminates = true;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-08  8:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-28  7:13 [Qemu-devel] memory: Add function pointers checks to memory_region_read/write() Salva Peiró
2015-07-28  7:13 ` [Qemu-devel] [PATCH] " Salva Peiró
  -- strict thread matches above, loose matches on Subject: below --
2015-09-03 17:37 Salva Peiró
2015-09-07 10:27 ` Paolo Bonzini
2015-09-08  6:51   ` Salva Peiró
2015-09-08  8:55     ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).