[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 02/12] storage: avoid mishandling backing store > 2GB



2011/6/6 Eric Blake <eblake redhat com>:
> Detected by Coverity.  The code was doing math on shifted unsigned
> char (which promotes to int), then promoting that to unsigned long
> during assignment to size.  On 64-bit platforms, this risks sign
> extending values of size > 2GiB.  Bug present since commit
> 489fd3 (v0.6.0).
>
> I'm not sure if a specially-crafted bogus qcow2 image could
> exploit this, although it's probably not possible, since we
> were already checking for the computed results being within
> range of our fixed-size buffer.
>
> * src/util/storage_file.c (qcowXGetBackingStore): Avoid sign
> extension.
> ---
>  src/util/storage_file.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/src/util/storage_file.c b/src/util/storage_file.c
> index 6b3b756..6749599 100644
> --- a/src/util/storage_file.c
> +++ b/src/util/storage_file.c
> @@ -27,6 +27,7 @@
>  #include <sys/stat.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> +#include <stdint.h>
>  #ifdef __linux__
>  # if HAVE_LINUX_MAGIC_H
>  #  include <linux/magic.h>
> @@ -274,7 +275,7 @@ qcowXGetBackingStore(char **res,
>                      bool isQCow2)
>  {
>     unsigned long long offset;
> -    unsigned long size;
> +    uint32_t size;
>
>     *res = NULL;
>     if (format)

Using unsigned int instead of uint32_t would also work and avoid
stdint.h types that the libvirt codebase avoids.

At any rate, ACK.

Matthias


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]