On Fri, Apr 04, 2014 at 11:51:52AM +0200, Juan Quintela wrote:
> "Michael S. Tsirkin" <[email protected]> wrote:
> > CVE-2013-4527 hw/timer/hpet.c buffer overrun
> >
> > hpet is a VARRAY with a uint8 size but static array of 32
> >
> > To fix, make sure num_timers is valid using VMSTATE_VALID hook.
> >
> > Reported-by: Anthony Liguori <[email protected]>
> > Signed-off-by: Michael S. Tsirkin <[email protected]>
> > Reviewed-by: Dr. David Alan Gilbert <[email protected]>
>
>
> Ok, seing what you want to do with VMSTATE_VALIDATE.
It does not seem clean to add a callback that is only
be used with a single type.
> Much better solution is add a ->validate() field, and use it in the
> equivalent of LESS_EQUAL and rest of tests.
>
> Will sent a patch.
>
> Later, Juan.
I think it's better to stick to plain C, extending macros like
LESS_EQUAL is too tricky, vmstate is already full ugly one-off macros.
> > ---
> > hw/timer/hpet.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> > index e15d6bc..2792f89 100644
> > --- a/hw/timer/hpet.c
> > +++ b/hw/timer/hpet.c
> > @@ -239,6 +239,18 @@ static int hpet_pre_load(void *opaque)
> > return 0;
> > }
> >
> > +static bool hpet_validate_num_timers(void *opaque, int version_id)
> > +{
> > + HPETState *s = opaque;
> > +
> > + if (s->num_timers < HPET_MIN_TIMERS) {
> > + return false;
> > + } else if (s->num_timers > HPET_MAX_TIMERS) {
> > + return false;
> > + }
> > + return true;
> > +}
> > +
> > static int hpet_post_load(void *opaque, int version_id)
> > {
> > HPETState *s = opaque;
> > @@ -307,6 +319,7 @@ static const VMStateDescription vmstate_hpet = {
> > VMSTATE_UINT64(isr, HPETState),
> > VMSTATE_UINT64(hpet_counter, HPETState),
> > VMSTATE_UINT8_V(num_timers, HPETState, 2),
> > + VMSTATE_VALIDATE("num_timers in range", hpet_validate_num_timers),
> > VMSTATE_STRUCT_VARRAY_UINT8(timer, HPETState, num_timers, 0,
> > vmstate_hpet_timer, HPETTimer),
> > VMSTATE_END_OF_LIST()
