[Xen-devel] Re: [PATCH 1 of 3 V3] tools/libxc: Remus Checkpoint Compression

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

rshriram@xxxxxxxxx writes ("[PATCH 1 of 3 V3] tools/libxc: Remus Checkpoint 
Compression"):
> +            flag = *src & FLAGMASK;
> +            len = *src & LENMASK;
> +            pos++;
> +            src++;
> +
> +            if (flag == RUNFLAG)
> +            {
> +                if ((pos + len * sizeof(uint32_t)) > compbuf_size)
> +                {
> +                    ERROR("Out of bounds exception in compression buffer 
> (d):"
> +                          "read ptr %lu, runlen = %u, bufsize = %lu\n",
> +                          pos, len * sizeof(uint32_t), compbuf_size);
> +                    return -1;
> +                }
> +
> +                memcpy(&destpage[pagepos], src, len * sizeof(uint32_t));

I think this may have a buffer overrun vulnerability.  It seems to me
that it can write beyond destpage + XC_PAGE_SIZE.  Decompressors are
often a source of security vulnerabilities of this kind, so it's
important that we look at it closely.

> +int xc_compression_uncompress_page(xc_interface *xch, char *compbuf,
> +                                   unsigned long compbuf_size,
> +                                   unsigned long *compbuf_pos, char *dest)
> +{
> +    return uncompress_page(xch, dest, compbuf_pos,
> +                           (uint8_t *)compbuf, compbuf_size);
> +}

What is the purpose of this wrapper function ?  All it seems to do is
massage the type and order of arguments.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel