[Qemu-devel] [PATCH v2] elf-ops.h: fix int overflow in load_elf()

[Qemu-devel] [PATCH v2] elf-ops.h: fix int overflow in load_elf()

[Stefano Garzarella]

This patch fixes a possible integer overflow when we calculate
the total size of ELF segments loaded.

Reported-by: Coverity (CID 1405299)
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
v2:
 - not use error_report in load_elf() [Peter]
 - return ELF_LOAD_TOO_BIG
 - add Alex's R-b
---
 include/hw/elf_ops.h | 5 +++++
 include/hw/loader.h  | 1 +
 hw/core/loader.c     | 2 ++
 3 files changed, 8 insertions(+)

diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 1496d7e753..e07d276df7 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -485,6 +485,11 @@ static int glue(load_elf, SZ)(const char *name, int fd,
                 }
             }
 
+            if (mem_size > INT_MAX - total_size) {
+                ret = ELF_LOAD_TOO_BIG;
+                goto fail;
+            }
+
             /* address_offset is hack for kernel images that are
                linked at the wrong physical address.  */
             if (translate_fn) {
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 07fd9286e7..48a96cd559 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -89,6 +89,7 @@ int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz);
 #define ELF_LOAD_NOT_ELF      -2
 #define ELF_LOAD_WRONG_ARCH   -3
 #define ELF_LOAD_WRONG_ENDIAN -4
+#define ELF_LOAD_TOO_BIG      -5
 const char *load_elf_strerror(int error);
 
 /** load_elf_ram_sym:
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 32f7cc7c33..75eb56ddbb 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -338,6 +338,8 @@ const char *load_elf_strerror(int error)
         return "The image is from incompatible architecture";
     case ELF_LOAD_WRONG_ENDIAN:
         return "The image has incorrect endianness";
+    case ELF_LOAD_TOO_BIG:
+        return "The image segments are too big to load";
     default:
         return "Unknown error";
     }
-- 
2.21.0

Re: [Qemu-devel] [PATCH v2] elf-ops.h: fix int overflow in load_elf()

[Paolo Bonzini]

On 10/09/19 14:48, Stefano Garzarella wrote:
> This patch fixes a possible integer overflow when we calculate
> the total size of ELF segments loaded.
> 
> Reported-by: Coverity (CID 1405299)
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> v2:
>  - not use error_report in load_elf() [Peter]
>  - return ELF_LOAD_TOO_BIG
>  - add Alex's R-b
> ---
>  include/hw/elf_ops.h | 5 +++++
>  include/hw/loader.h  | 1 +
>  hw/core/loader.c     | 2 ++
>  3 files changed, 8 insertions(+)
> 
> diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
> index 1496d7e753..e07d276df7 100644
> --- a/include/hw/elf_ops.h
> +++ b/include/hw/elf_ops.h
> @@ -485,6 +485,11 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>                  }
>              }
>  
> +            if (mem_size > INT_MAX - total_size) {
> +                ret = ELF_LOAD_TOO_BIG;
> +                goto fail;
> +            }
> +
>              /* address_offset is hack for kernel images that are
>                 linked at the wrong physical address.  */
>              if (translate_fn) {
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 07fd9286e7..48a96cd559 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -89,6 +89,7 @@ int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz);
>  #define ELF_LOAD_NOT_ELF      -2
>  #define ELF_LOAD_WRONG_ARCH   -3
>  #define ELF_LOAD_WRONG_ENDIAN -4
> +#define ELF_LOAD_TOO_BIG      -5
>  const char *load_elf_strerror(int error);
>  
>  /** load_elf_ram_sym:
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 32f7cc7c33..75eb56ddbb 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -338,6 +338,8 @@ const char *load_elf_strerror(int error)
>          return "The image is from incompatible architecture";
>      case ELF_LOAD_WRONG_ENDIAN:
>          return "The image has incorrect endianness";
> +    case ELF_LOAD_TOO_BIG:
> +        return "The image segments are too big to load";
>      default:
>          return "Unknown error";
>      }
> 

Queued, thanks.

Paolo