Keith Busch <keith.bu...@intel.com> 于2018年11月2日周五 下午11:42写道：
> 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 <liq...@gmail.com> > > Reviewed-by: Paolo Bonzini <pbonz...@redhat.com> > > Signed-off-by: Li Qiang <liq...@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? > > Yes, This seems a good idea. Once I also encounter one issue like this. The read callback in MR will be called even it is NULL so cause a NULL-deref. discussed here: -->https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg01391.html This touchs the core design of memory subsystem I think, May Paolo or Peter can answer this. > 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, > -- >
