Message ID | 1386087086-3691-10-git-send-email-mst@redhat.com |
---|---|
State | New |
Headers | show |
On 3 December 2013 16:28, Michael S. Tsirkin <mst@redhat.com> wrote: > CVE-2013-4531 > > cpreg_vmstate_indexes is a VARRAY_INT32. A negative value for > cpreg_vmstate_array_len will cause a buffer overflow. > > Reported-by: Anthony Liguori <anthony@codemonkey.ws> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > target-arm/machine.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/target-arm/machine.c b/target-arm/machine.c > index 74f010f..d46b7e8 100644 > --- a/target-arm/machine.c > +++ b/target-arm/machine.c > @@ -178,6 +178,10 @@ static int cpu_post_load(void *opaque, int version_id) > ARMCPU *cpu = opaque; > int i, v; > > + if (cpu->cpreg_vmstate_array_len < 0) { > + return -1; > + } > + I think this is the wrong way to fix this bug. The intent of the code is that using VMSTATE_INT32_LE() for the array_len makes the migration vmstate code do a check and reject array lengths which overflow the array. We should fix this check rather than attempting to catch the cases where it didn't work in the post_load hook, not least because by the time we've reached the post-load hook we'll have already overrun the buffer... Given the uses of VMSTATE_INT32_LE I suspect we could safely redefine its semantics to mean "only OK if less-than-or-equal to X and non-negative". Or we could have a new VMSTATE_ macro with those semantics, for array-length checks. thanks -- PMM
diff --git a/target-arm/machine.c b/target-arm/machine.c index 74f010f..d46b7e8 100644 --- a/target-arm/machine.c +++ b/target-arm/machine.c @@ -178,6 +178,10 @@ static int cpu_post_load(void *opaque, int version_id) ARMCPU *cpu = opaque; int i, v; + if (cpu->cpreg_vmstate_array_len < 0) { + return -1; + } + /* Update the values list from the incoming migration data. * Anything in the incoming data which we don't know about is * a migration failure; anything we know about but the incoming
CVE-2013-4531 cpreg_vmstate_indexes is a VARRAY_INT32. A negative value for cpreg_vmstate_array_len will cause a buffer overflow. Reported-by: Anthony Liguori <anthony@codemonkey.ws> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- target-arm/machine.c | 4 ++++ 1 file changed, 4 insertions(+)