Message ID | 1396275242-10810-18-git-send-email-mst@redhat.com |
---|---|
State | New |
Headers | show |
On 31 March 2014 15:17, Michael S. Tsirkin <mst@redhat.com> wrote: > From: Michael Roth <mdroth@linux.vnet.ibm.com> > > CVE-2013-4534 > > opp->nb_cpus is read from the wire and used to determine how many > IRQDest elements to read into opp->dst[]. If the value exceeds the > length of opp->dst[], MAX_CPU, opp->dst[] can be overrun with arbitrary > data from the wire. > > Fix this by failing migration if the value read from the wire exceeds > MAX_CPU. > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/intc/openpic.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c > index be76fbd..8cb16da 100644 > --- a/hw/intc/openpic.c > +++ b/hw/intc/openpic.c > @@ -1429,6 +1429,9 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id) > qemu_get_be32s(f, &opp->tfrr); > > qemu_get_be32s(f, &opp->nb_cpus); > + if (opp->nb_cpus > MAX_CPU) { > + return -EINVAL; > + } nb_cpus is a device property so we shouldn't be reading it off the wire at all. I would suggest treating it as a "read but ignore value", assuming we care about migration back-compatibility for this device, or possibly "read and confirm it matches the value we already have". thanks -- PMM
diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c index be76fbd..8cb16da 100644 --- a/hw/intc/openpic.c +++ b/hw/intc/openpic.c @@ -1429,6 +1429,9 @@ static int openpic_load(QEMUFile* f, void *opaque, int version_id) qemu_get_be32s(f, &opp->tfrr); qemu_get_be32s(f, &opp->nb_cpus); + if (opp->nb_cpus > MAX_CPU) { + return -EINVAL; + } for (i = 0; i < opp->nb_cpus; i++) { qemu_get_sbe32s(f, &opp->dst[i].ctpr);