Am 02.11.2018 um 16:22 hat Li Qiang geschrieben:
> Hello Kevin,
>
> Kevin Wolf <address@hidden> 于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 <address@hidden>
> > > Reviewed-by: Paolo Bonzini <address@hidden>
> > > Signed-off-by: Li Qiang <address@hidden>
> > > ---
> > > 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?
I think it can't happen, as the 'addr' here is the relative offset in the MR.
As we don't (can't) register such a MMIO region(the end of this region is very big to
make addr+size overflow, size here can only be 1,2, 4, 8,). So the 'addr' here can't be that large.
Thanks,
Li Qiang
(As Keith confirmed, the integer overflow doesn't seem to have any bad
consequences here, but anyway.)
Kevin