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

[Kevin Wolf <kwolf@redhat.com>]

Am 02.11.2018 um 16:22 hat Li Qiang geschrieben:
> Hello Kevin,
> 
> Kevin Wolf <kwolf@redhat.com> 于2018年11月2日周五 下午6:54写道：
> 
> > Am 02.11.2018 um 02:22 hat Li Qiang geschrieben:
> > > 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>
> > > ---
> > >  hw/block/nvme.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > > index fc7dacb..d097add 100644
> > > --- a/hw/block/nvme.c
> > > +++ b/hw/block/nvme.c
> > > @@ -1175,6 +1175,10 @@ static void nvme_cmb_write(void *opaque, hwaddr
> > addr, uint64_t data,
> > >      unsigned size)
> > >  {
> > >      NvmeCtrl *n = (NvmeCtrl *)opaque;
> > > +
> > > +    if (addr + size > NVME_CMBSZ_GETSIZE(n->bar.cmbsz)) {
> >
> > What prevents a guest from moving the device to the end of the address
> > space and causing an integer overflow in addr + size?
> >
> >
> This can't happen as the addr can't be any value, it just can be in the
> Memory Region n->ctrl_mem defines.

Yes, but can't the guest map that memory region whereever it wants?

(As Keith confirmed, the integer overflow doesn't seem to have any bad
consequences here, but anyway.)

Kevin