Re: [Xen-devel] Xen Security Advisory 27 (CVE-2012-5511) - several HVM operations do not validate the range of their inputs

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.01.13 at 11:23, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Sat, 2013-01-12 at 15:35 +0000, Matt Wilson wrote:
>> > diff -r 5639047d6c9f xen/arch/x86/mm/paging.c
>> > --- a/xen/arch/x86/mm/paging.c     Mon Nov 19 09:43:48 2012 +0100
>> > +++ b/xen/arch/x86/mm/paging.c     Mon Nov 19 16:00:33 2012 +0000
>> > @@ -529,13 +529,18 @@ int paging_log_dirty_range(struct domain
>> >  
>> >      if ( !d->arch.paging.log_dirty.fault_count &&
>> >           !d->arch.paging.log_dirty.dirty_count ) {
>> > -        int size = (nr + BITS_PER_LONG - 1) / BITS_PER_LONG;
>> > -        unsigned long zeroes[size];
>> > -        memset(zeroes, 0x00, size * BYTES_PER_LONG);
>> > +        static uint8_t zeroes[PAGE_SIZE];
>> > +        int off, size;
>> > +
>> > +        size = ((nr + BITS_PER_LONG - 1) / BITS_PER_LONG) * sizeof (long);
>> >          rv = 0;
>> > -        if ( copy_to_guest_offset(dirty_bitmap, 0, (uint8_t *) zeroes,
>> > -                                  size * BYTES_PER_LONG) != 0 )
>> > -            rv = -EFAULT;
>> > +        for ( off = 0; !rv && off < size; off += sizeof(zeroes) )
>> > +        {
>> > +            int todo = min(size - off, (int) PAGE_SIZE);
>> > +            if ( copy_to_guest_offset(dirty_bitmap, off, zeroes, todo) )
>> > +                rv = -EFAULT;
>> > +            off += todo;
>> 
>> off is incremented twice, once as part of the for loop and once
>> inside. Was that intended?
> 
> It certainly does seem wrong (or too clever for me).
> 
> I think either could correctly be removed but the more logical one would
> be the one in the for loop, I think, since the one inside the body is
> more accurate (although it only matters for the final iteration and
> either would cause the loop to exit).

I agree, but since it was Tim who had put this on-off together
(as a smaller replacement to the fix that went into 4.2 before
its release), I'd leave this to him.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel