Re: [Qemu-devel] [PATCH] nvme: fix oob access issue(CVE-2018-16847)

[Keith Busch <keith.busch@intel.com>]

On Thu, Nov 01, 2018 at 06:22:43PM -0700, Li Qiang wrote:
> Currently, the nvme_cmb_ops mr doesn't check the addr and size.
> This can lead an oob access issue. This is triggerable in the guest.
> Add check to avoid this issue.
> 
> Fixes CVE-2018-16847.
> 
> Reported-by: Li Qiang <liq3ea@gmail.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Li Qiang <liq3ea@gmail.com>

Hey, so why is this memory region access even considered valid if the
request is out of range from what NVMe had registered for its
MemoryRegion? Wouldn't it be better to not call the mr->ops->read/write
if it's out of bounds? Otherwise every MemoryRegion needs to duplicate
the same check, right?

Would something like the following work (minimally tested)?

---
diff --git a/memory.c b/memory.c
index 9b73892768..883fd818e6 100644
--- a/memory.c
+++ b/memory.c
@@ -1369,6 +1369,9 @@ bool memory_region_access_valid(MemoryRegion *mr,
         access_size_max = 4;
     }
 
+    if (addr + size > mr->size)
+        return false;
+
     access_size = MAX(MIN(size, access_size_max), access_size_min);
     for (i = 0; i < size; i += access_size) {
         if (!mr->ops->valid.accepts(mr->opaque, addr + i, access_size,
--