On 03.04.2014, at 18:51, Michael S. Tsirkin <m...@redhat.com> wrote:
> From: Michael Roth <mdr...@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 <mdr...@linux.vnet.ibm.com>
> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
Reviewed-by: Alexander Graf <ag...@suse.de>
Though I don't think the nb_cpus checks are necessary, it's always nice to be
safe rather than sorry :).
Alex
> ---
> hw/intc/openpic.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c
> index be76fbd..e63ccf2 100644
> --- a/hw/intc/openpic.c
> +++ b/hw/intc/openpic.c
> @@ -1416,7 +1416,7 @@ static void openpic_load_IRQ_queue(QEMUFile* f,
> IRQQueue *q)
> static int openpic_load(QEMUFile* f, void *opaque, int version_id)
> {
> OpenPICState *opp = (OpenPICState *)opaque;
> - unsigned int i;
> + unsigned int i, nb_cpus;
>
> if (version_id != 1) {
> return -EINVAL;
> @@ -1428,7 +1428,11 @@ static int openpic_load(QEMUFile* f, void *opaque, int
> version_id)
> qemu_get_be32s(f, &opp->spve);
> qemu_get_be32s(f, &opp->tfrr);
>
> - qemu_get_be32s(f, &opp->nb_cpus);
> + qemu_get_be32s(f, &nb_cpus);
> + if (opp->nb_cpus != nb_cpus) {
> + return -EINVAL;
> + }
> + assert(nb_cpus > 0 && nb_cpus <= MAX_CPU);
>
> for (i = 0; i < opp->nb_cpus; i++) {
> qemu_get_sbe32s(f, &opp->dst[i].ctpr);
> @@ -1567,6 +1571,12 @@ static void openpic_realize(DeviceState *dev, Error
> **errp)
> {NULL}
> };
>
> + if (opp->nb_cpus > MAX_CPU) {
> + error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
> + TYPE_OPENPIC, "nb_cpus", 0, MAX_CPU);
> + return;
> + }
> +
> switch (opp->model) {
> case OPENPIC_MODEL_FSL_MPIC_20:
> default:
> --
> MST
>
