diff mbox

[2/3] Allow larger return values from get_image_size()

Message ID 1330043806-31899-3-git-send-email-david@gibson.dropbear.id.au
State New
Headers show

Commit Message

David Gibson Feb. 24, 2012, 12:36 a.m. UTC 
Currently get_image_size(), used to find the size of files, returns an int.
But for modern systems, int may be only 32-bit and we can have files
larger than that.

This patch, therefore, changes the return type of get_image_size() to off_t
(the same as the return type from lseek() itself).  It also audits all the
callers of get_image_size() to make sure they process the new unsigned
return type correctly.

This leaves load_image_targphys() with a limited return type, but one thing
at a time (that function has far more callers to be audited, so it will
take longer to fix).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 device_tree.c      |    4 ++--
 hw/alpha_dp264.c   |    5 +++--
 hw/highbank.c      |    2 +-
 hw/leon3.c         |    6 +++---
 hw/loader.c        |   12 +++++++-----
 hw/loader.h        |    2 +-
 hw/mips_fulong2e.c |   12 ++++++------
 hw/mips_malta.c    |   12 ++++++------
 hw/mips_mipssim.c  |    6 +++---
 hw/mips_r4k.c      |   17 +++++++++--------
 hw/multiboot.c     |    4 ++--
 hw/palm.c          |   11 ++++++-----
 hw/pc.c            |    5 +++--
 hw/pc_sysfw.c      |    6 +++---
 hw/pci.c           |    4 ++--
 hw/ppc_prep.c      |    7 ++++---
 hw/smbios.c        |    2 +-
 17 files changed, 62 insertions(+), 55 deletions(-)

Comments

Michael S. Tsirkin Feb. 24, 2012, 12:48 a.m. UTC | #1 
On Fri, Feb 24, 2012 at 11:36:45AM +1100, David Gibson wrote:
> Currently get_image_size(), used to find the size of files, returns an int.
> But for modern systems, int may be only 32-bit and we can have files
> larger than that.
> 
> This patch, therefore, changes the return type of get_image_size() to off_t
> (the same as the return type from lseek() itself).  It also audits all the
> callers of get_image_size() to make sure they process the new unsigned
> return type correctly.
> 
> This leaves load_image_targphys() with a limited return type, but one thing
> at a time (that function has far more callers to be audited, so it will
> take longer to fix).
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Hmm, this can still be 32 bit.
We probably should set 

#define_FILE_OFFSET_BITS 64

before including any standard headers.
This will make it 64 bit consistently.

> ---
>  device_tree.c      |    4 ++--
>  hw/alpha_dp264.c   |    5 +++--
>  hw/highbank.c      |    2 +-
>  hw/leon3.c         |    6 +++---
>  hw/loader.c        |   12 +++++++-----
>  hw/loader.h        |    2 +-
>  hw/mips_fulong2e.c |   12 ++++++------
>  hw/mips_malta.c    |   12 ++++++------
>  hw/mips_mipssim.c  |    6 +++---
>  hw/mips_r4k.c      |   17 +++++++++--------
>  hw/multiboot.c     |    4 ++--
>  hw/palm.c          |   11 ++++++-----
>  hw/pc.c            |    5 +++--
>  hw/pc_sysfw.c      |    6 +++---
>  hw/pci.c           |    4 ++--
>  hw/ppc_prep.c      |    7 ++++---
>  hw/smbios.c        |    2 +-
>  17 files changed, 62 insertions(+), 55 deletions(-)
> 
> diff --git a/device_tree.c b/device_tree.c
> index 86a694c..5817b2d 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -27,14 +27,14 @@
>  
>  void *load_device_tree(const char *filename_path, int *sizep)
>  {
> -    int dt_size;
> +    off_t dt_size;
>      int dt_file_load_size;
>      int ret;
>      void *fdt = NULL;
>  
>      *sizep = 0;
>      dt_size = get_image_size(filename_path);
> -    if (dt_size < 0) {
> +    if (dt_size == -1) {
>          printf("Unable to get size of device tree file '%s'\n",
>              filename_path);
>          goto fail;
> diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c
> index ea0fd95..01ec9c7 100644
> --- a/hw/alpha_dp264.c
> +++ b/hw/alpha_dp264.c
> @@ -144,10 +144,11 @@ static void clipper_init(ram_addr_t ram_size,
>          }
>  
>          if (initrd_filename) {
> -            long initrd_base, initrd_size;
> +            long initrd_base;
> +            off_t initrd_size;
>  
>              initrd_size = get_image_size(initrd_filename);
> -            if (initrd_size < 0) {
> +            if (initrd_size != -1) {
>                  hw_error("could not load initial ram disk '%s'\n",
>                           initrd_filename);
>                  exit(1);
> diff --git a/hw/highbank.c b/hw/highbank.c
> index 489c00e..f4de4c6 100644
> --- a/hw/highbank.c
> +++ b/hw/highbank.c
> @@ -235,7 +235,7 @@ static void highbank_init(ram_addr_t ram_size,
>      if (bios_name != NULL) {
>          sysboot_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>          if (sysboot_filename != NULL) {
> -            uint32_t filesize = get_image_size(sysboot_filename);
> +            off_t filesize = get_image_size(sysboot_filename);
>              if (load_image_targphys("sysram.bin", 0xfff88000, filesize) < 0) {
>                  hw_error("Unable to load %s\n", bios_name);
>              }
> diff --git a/hw/leon3.c b/hw/leon3.c
> index 71d79a6..f7d4860 100644
> --- a/hw/leon3.c
> +++ b/hw/leon3.c
> @@ -108,7 +108,7 @@ static void leon3_generic_hw_init(ram_addr_t  ram_size,
>      int         ret;
>      char       *filename;
>      qemu_irq   *cpu_irqs = NULL;
> -    int         bios_size;
> +    off_t       bios_size;
>      int         prom_size;
>      ResetData  *reset_info;
>  
> @@ -168,7 +168,7 @@ static void leon3_generic_hw_init(ram_addr_t  ram_size,
>          exit(1);
>      }
>  
> -    if (bios_size > 0) {
> +    if (bios_size != -1) {
>          ret = load_image_targphys(filename, 0x00000000, bios_size);
>          if (ret < 0 || ret > prom_size) {
>              fprintf(stderr, "qemu: could not load prom '%s'\n", filename);
> @@ -191,7 +191,7 @@ static void leon3_generic_hw_init(ram_addr_t  ram_size,
>                      kernel_filename);
>              exit(1);
>          }
> -        if (bios_size <= 0) {
> +        if (bios_size == 0 || bios_size == -1) {
>              /* If there is no bios/monitor, start the application.  */
>              env->pc = entry;
>              env->npc = entry + 4;
> diff --git a/hw/loader.c b/hw/loader.c
> index 415cdce..372f6f8 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -57,12 +57,14 @@
>  static int roms_loaded;
>  
>  /* return the size or -1 if error */
> -int get_image_size(const char *filename)
> +off_t get_image_size(const char *filename)
>  {
> -    int fd, size;
> +    int fd;
> +    off_t size;
> +
>      fd = open(filename, O_RDONLY | O_BINARY);
>      if (fd < 0)
> -        return -1;
> +        return (off_t)-1;
>      size = lseek(fd, 0, SEEK_END);
>      close(fd);
>      return size;
> @@ -105,13 +107,13 @@ ssize_t read_targphys(const char *name,
>  int load_image_targphys(const char *filename,
>  			target_phys_addr_t addr, int max_sz)
>  {
> -    int size;
> +    off_t size;
>  
>      size = get_image_size(filename);
>      if (size > max_sz) {
>          return -1;
>      }
> -    if (size > 0) {
> +    if (size != -1) {
>          rom_add_file_fixed(filename, addr, -1);
>      }
>      return size;
> diff --git a/hw/loader.h b/hw/loader.h
> index fbcaba9..1fbdbcf 100644
> --- a/hw/loader.h
> +++ b/hw/loader.h
> @@ -2,7 +2,7 @@
>  #define LOADER_H
>  
>  /* loader.c */
> -int get_image_size(const char *filename);
> +off_t get_image_size(const char *filename);
>  int load_image(const char *filename, uint8_t *addr); /* deprecated */
>  int load_image_targphys(const char *filename, target_phys_addr_t, int max_sz);
>  int load_elf(const char *filename, uint64_t (*translate_fn)(void *, uint64_t),
> diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c
> index e3ba9dd..1e7b49a 100644
> --- a/hw/mips_fulong2e.c
> +++ b/hw/mips_fulong2e.c
> @@ -107,7 +107,7 @@ static int64_t load_kernel (CPUState *env)
>  {
>      int64_t kernel_entry, kernel_low, kernel_high;
>      int index = 0;
> -    long initrd_size;
> +    off_t initrd_size;
>      ram_addr_t initrd_offset;
>      uint32_t *prom_buf;
>      long prom_size;
> @@ -125,7 +125,7 @@ static int64_t load_kernel (CPUState *env)
>      initrd_offset = 0;
>      if (loaderparams.initrd_filename) {
>          initrd_size = get_image_size (loaderparams.initrd_filename);
> -        if (initrd_size > 0) {
> +        if (initrd_size != -1) {
>              initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) & TARGET_PAGE_MASK;
>              if (initrd_offset + initrd_size > ram_size) {
>                  fprintf(stderr,
> @@ -136,7 +136,7 @@ static int64_t load_kernel (CPUState *env)
>              initrd_size = load_image_targphys(loaderparams.initrd_filename,
>                                       initrd_offset, ram_size - initrd_offset);
>          }
> -        if (initrd_size == (target_ulong) -1) {
> +        if (initrd_size == -1) {
>              fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
>                      loaderparams.initrd_filename);
>              exit(1);
> @@ -148,10 +148,10 @@ static int64_t load_kernel (CPUState *env)
>      prom_buf = g_malloc(prom_size);
>  
>      prom_set(prom_buf, index++, "%s", loaderparams.kernel_filename);
> -    if (initrd_size > 0) {
> +    if (initrd_size != -1) {
>          prom_set(prom_buf, index++, "rd_start=0x%" PRIx64 " rd_size=%li %s",
> -                 cpu_mips_phys_to_kseg0(NULL, initrd_offset), initrd_size,
> -                 loaderparams.kernel_cmdline);
> +                 cpu_mips_phys_to_kseg0(NULL, initrd_offset),
> +                 (long)initrd_size, loaderparams.kernel_cmdline);
>      } else {
>          prom_set(prom_buf, index++, "%s", loaderparams.kernel_cmdline);
>      }
> diff --git a/hw/mips_malta.c b/hw/mips_malta.c
> index 86a5fbb..50e800a 100644
> --- a/hw/mips_malta.c
> +++ b/hw/mips_malta.c
> @@ -667,7 +667,7 @@ static void GCC_FMT_ATTR(3, 4) prom_set(uint32_t* prom_buf, int index,
>  static int64_t load_kernel (void)
>  {
>      int64_t kernel_entry, kernel_high;
> -    long initrd_size;
> +    off_t initrd_size;
>      ram_addr_t initrd_offset;
>      int big_endian;
>      uint32_t *prom_buf;
> @@ -693,7 +693,7 @@ static int64_t load_kernel (void)
>      initrd_offset = 0;
>      if (loaderparams.initrd_filename) {
>          initrd_size = get_image_size (loaderparams.initrd_filename);
> -        if (initrd_size > 0) {
> +        if (initrd_size != -1) {
>              initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) & TARGET_PAGE_MASK;
>              if (initrd_offset + initrd_size > ram_size) {
>                  fprintf(stderr,
> @@ -705,7 +705,7 @@ static int64_t load_kernel (void)
>                                                initrd_offset,
>                                                ram_size - initrd_offset);
>          }
> -        if (initrd_size == (target_ulong) -1) {
> +        if (initrd_size == -1) {
>              fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
>                      loaderparams.initrd_filename);
>              exit(1);
> @@ -717,10 +717,10 @@ static int64_t load_kernel (void)
>      prom_buf = g_malloc(prom_size);
>  
>      prom_set(prom_buf, prom_index++, "%s", loaderparams.kernel_filename);
> -    if (initrd_size > 0) {
> +    if (initrd_size != -1) {
>          prom_set(prom_buf, prom_index++, "rd_start=0x%" PRIx64 " rd_size=%li %s",
> -                 cpu_mips_phys_to_kseg0(NULL, initrd_offset), initrd_size,
> -                 loaderparams.kernel_cmdline);
> +                 cpu_mips_phys_to_kseg0(NULL, initrd_offset),
> +                 (long)initrd_size, loaderparams.kernel_cmdline);
>      } else {
>          prom_set(prom_buf, prom_index++, "%s", loaderparams.kernel_cmdline);
>      }
> diff --git a/hw/mips_mipssim.c b/hw/mips_mipssim.c
> index 76c95b2..89de6ca 100644
> --- a/hw/mips_mipssim.c
> +++ b/hw/mips_mipssim.c
> @@ -54,7 +54,7 @@ static int64_t load_kernel(void)
>  {
>      int64_t entry, kernel_high;
>      long kernel_size;
> -    long initrd_size;
> +    off_t initrd_size;
>      ram_addr_t initrd_offset;
>      int big_endian;
>  
> @@ -82,7 +82,7 @@ static int64_t load_kernel(void)
>      initrd_offset = 0;
>      if (loaderparams.initrd_filename) {
>          initrd_size = get_image_size (loaderparams.initrd_filename);
> -        if (initrd_size > 0) {
> +        if (initrd_size != -1) {
>              initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) & TARGET_PAGE_MASK;
>              if (initrd_offset + initrd_size > loaderparams.ram_size) {
>                  fprintf(stderr,
> @@ -93,7 +93,7 @@ static int64_t load_kernel(void)
>              initrd_size = load_image_targphys(loaderparams.initrd_filename,
>                  initrd_offset, loaderparams.ram_size - initrd_offset);
>          }
> -        if (initrd_size == (target_ulong) -1) {
> +        if (initrd_size == -1) {
>              fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
>                      loaderparams.initrd_filename);
>              exit(1);
> diff --git a/hw/mips_r4k.c b/hw/mips_r4k.c
> index 83401f0..067a793 100644
> --- a/hw/mips_r4k.c
> +++ b/hw/mips_r4k.c
> @@ -72,7 +72,8 @@ typedef struct ResetData {
>  static int64_t load_kernel(void)
>  {
>      int64_t entry, kernel_high;
> -    long kernel_size, initrd_size, params_size;
> +    long kernel_size, params_size;
> +    off_t initrd_size;
>      ram_addr_t initrd_offset;
>      uint32_t *params_buf;
>      int big_endian;
> @@ -100,7 +101,7 @@ static int64_t load_kernel(void)
>      initrd_offset = 0;
>      if (loaderparams.initrd_filename) {
>          initrd_size = get_image_size (loaderparams.initrd_filename);
> -        if (initrd_size > 0) {
> +        if (initrd_size != -1) {
>              initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) & TARGET_PAGE_MASK;
>              if (initrd_offset + initrd_size > ram_size) {
>                  fprintf(stderr,
> @@ -112,7 +113,7 @@ static int64_t load_kernel(void)
>                                                initrd_offset,
>                                                ram_size - initrd_offset);
>          }
> -        if (initrd_size == (target_ulong) -1) {
> +        if (initrd_size == -1) {
>              fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
>                      loaderparams.initrd_filename);
>              exit(1);
> @@ -126,10 +127,10 @@ static int64_t load_kernel(void)
>      params_buf[0] = tswap32(ram_size);
>      params_buf[1] = tswap32(0x12345678);
>  
> -    if (initrd_size > 0) {
> +    if (initrd_size != -1) {
>          snprintf((char *)params_buf + 8, 256, "rd_start=0x%" PRIx64 " rd_size=%li %s",
>                   cpu_mips_phys_to_kseg0(NULL, initrd_offset),
> -                 initrd_size, loaderparams.kernel_cmdline);
> +                 (long)initrd_size, loaderparams.kernel_cmdline);
>      } else {
>          snprintf((char *)params_buf + 8, 256, "%s", loaderparams.kernel_cmdline);
>      }
> @@ -161,7 +162,7 @@ void mips_r4k_init (ram_addr_t ram_size,
>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>      MemoryRegion *bios;
>      MemoryRegion *iomem = g_new(MemoryRegion, 1);
> -    int bios_size;
> +    off_t bios_size;
>      CPUState *env;
>      ResetData *reset_info;
>      int i;
> @@ -214,14 +215,14 @@ void mips_r4k_init (ram_addr_t ram_size,
>      if (filename) {
>          bios_size = get_image_size(filename);
>      } else {
> -        bios_size = -1;
> +        bios_size = (off_t)-1;
>      }
>  #ifdef TARGET_WORDS_BIGENDIAN
>      be = 1;
>  #else
>      be = 0;
>  #endif
> -    if ((bios_size > 0) && (bios_size <= BIOS_SIZE)) {
> +    if ((bios_size != -1) && (bios_size <= BIOS_SIZE)) {
>          bios = g_new(MemoryRegion, 1);
>          memory_region_init_ram(bios, "mips_r4k.bios", BIOS_SIZE);
>          vmstate_register_ram_global(bios);
> diff --git a/hw/multiboot.c b/hw/multiboot.c
> index b4484a3..37e8af5 100644
> --- a/hw/multiboot.c
> +++ b/hw/multiboot.c
> @@ -262,7 +262,7 @@ int load_multiboot(void *fw_cfg,
>  
>          do {
>              char *next_space;
> -            int mb_mod_length;
> +            off_t mb_mod_length;
>              uint32_t offs = mbs.mb_buf_size;
>  
>              next_initrd = (char *)get_opt_value(NULL, 0, initrd_filename);
> @@ -275,7 +275,7 @@ int load_multiboot(void *fw_cfg,
>                  *next_space = '\0';
>              mb_debug("multiboot loading module: %s\n", initrd_filename);
>              mb_mod_length = get_image_size(initrd_filename);
> -            if (mb_mod_length < 0) {
> +            if (mb_mod_length == -1) {
>                  fprintf(stderr, "Failed to open file '%s'\n", initrd_filename);
>                  exit(1);
>              }
> diff --git a/hw/palm.c b/hw/palm.c
> index b1252ab..176ad4b 100644
> --- a/hw/palm.c
> +++ b/hw/palm.c
> @@ -203,7 +203,8 @@ static void palmte_init(ram_addr_t ram_size,
>      static uint32_t cs1val = 0x0000e1a0;
>      static uint32_t cs2val = 0x0000e1a0;
>      static uint32_t cs3val = 0xe1a0e1a0;
> -    int rom_size, rom_loaded = 0;
> +    int rom_loaded = 0;
> +    off_t rom_size;
>      DisplayState *ds = get_displaystate();
>      MemoryRegion *flash = g_new(MemoryRegion, 1);
>      MemoryRegion *cs = g_new(MemoryRegion, 4);
> @@ -240,16 +241,16 @@ static void palmte_init(ram_addr_t ram_size,
>      if (nb_option_roms) {
>          rom_size = get_image_size(option_rom[0].name);
>          if (rom_size > flash_size) {
> -            fprintf(stderr, "%s: ROM image too big (%x > %x)\n",
> -                            __FUNCTION__, rom_size, flash_size);
> +            fprintf(stderr, "%s: ROM image too big (%lx > %x)\n",
> +                    __FUNCTION__, (unsigned long)rom_size, flash_size);
>              rom_size = 0;
>          }
> -        if (rom_size > 0) {
> +        if (rom_size != -1) {
>              rom_size = load_image_targphys(option_rom[0].name, OMAP_CS0_BASE,
>                                             flash_size);
>              rom_loaded = 1;
>          }
> -        if (rom_size < 0) {
> +        if (rom_size == -1) {
>              fprintf(stderr, "%s: error loading '%s'\n",
>                              __FUNCTION__, option_rom[0].name);
>          }
> diff --git a/hw/pc.c b/hw/pc.c
> index b9f4bc7..cb41955 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -672,7 +672,8 @@ static void load_linux(void *fw_cfg,
>                         target_phys_addr_t max_ram_size)
>  {
>      uint16_t protocol;
> -    int setup_size, kernel_size, initrd_size = 0, cmdline_size;
> +    int setup_size, kernel_size, cmdline_size;
> +    off_t initrd_size = 0;
>      uint32_t initrd_max;
>      uint8_t header[8192], *setup, *kernel, *initrd_data;
>      target_phys_addr_t real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
> @@ -795,7 +796,7 @@ static void load_linux(void *fw_cfg,
>  	}
>  
>  	initrd_size = get_image_size(initrd_filename);
> -        if (initrd_size < 0) {
> +        if (initrd_size == -1) {
>              fprintf(stderr, "qemu: error reading initrd %s\n",
>                      initrd_filename);
>              exit(1);
> diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
> index abf9004..7e2f715 100644
> --- a/hw/pc_sysfw.c
> +++ b/hw/pc_sysfw.c
> @@ -132,7 +132,7 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory)
>  {
>      char *filename;
>      MemoryRegion *bios, *isa_bios;
> -    int bios_size, isa_bios_size;
> +    off_t bios_size, isa_bios_size;
>      int ret;
>  
>      /* BIOS load */
> @@ -143,9 +143,9 @@ static void old_pc_system_rom_init(MemoryRegion *rom_memory)
>      if (filename) {
>          bios_size = get_image_size(filename);
>      } else {
> -        bios_size = -1;
> +        bios_size = (off_t)-1;
>      }
> -    if (bios_size <= 0 ||
> +    if ((bios_size == 0) || (bios_size == -1) ||
>          (bios_size % 65536) != 0) {
>          goto bios_error;
>      }
> diff --git a/hw/pci.c b/hw/pci.c
> index bf046bf..5720a8c 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1672,7 +1672,7 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size)
>  /* Add an option rom for the device */
>  static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
>  {
> -    int size;
> +    off_t size;
>      char *path;
>      void *ptr;
>      char name[32];
> @@ -1703,7 +1703,7 @@ static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
>      }
>  
>      size = get_image_size(path);
> -    if (size < 0) {
> +    if (size == -1) {
>          error_report("%s: failed to find romfile \"%s\"",
>                       __FUNCTION__, pdev->romfile);
>          g_free(path);
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index eb43fb5..c41e13c 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -489,7 +489,8 @@ static void ppc_prep_init (ram_addr_t ram_size,
>  #if 0
>      MemoryRegion *xcsr = g_new(MemoryRegion, 1);
>  #endif
> -    int linux_boot, i, nb_nics1, bios_size;
> +    int linux_boot, i, nb_nics1;
> +    off_t bios_size;
>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>      MemoryRegion *bios = g_new(MemoryRegion, 1);
>      uint32_t kernel_base, initrd_base;
> @@ -546,13 +547,13 @@ static void ppc_prep_init (ram_addr_t ram_size,
>      } else {
>          bios_size = -1;
>      }
> -    if (bios_size > 0 && bios_size <= BIOS_SIZE) {
> +    if (bios_size != -1 && bios_size <= BIOS_SIZE) {
>          target_phys_addr_t bios_addr;
>          bios_size = (bios_size + 0xfff) & ~0xfff;
>          bios_addr = (uint32_t)(-bios_size);
>          bios_size = load_image_targphys(filename, bios_addr, bios_size);
>      }
> -    if (bios_size < 0 || bios_size > BIOS_SIZE) {
> +    if (bios_size == -1 || bios_size > BIOS_SIZE) {
>          hw_error("qemu: could not load PPC PREP bios '%s'\n", bios_name);
>      }
>      if (filename) {
> diff --git a/hw/smbios.c b/hw/smbios.c
> index c57237d..85fad0e 100644
> --- a/hw/smbios.c
> +++ b/hw/smbios.c
> @@ -185,7 +185,7 @@ int smbios_entry_add(const char *t)
>      if (get_param_value(buf, sizeof(buf), "file", t)) {
>          struct smbios_structure_header *header;
>          struct smbios_table *table;
> -        int size = get_image_size(buf);
> +        off_t size = get_image_size(buf);
>  
>          if (size == -1 || size < sizeof(struct smbios_structure_header)) {
>              fprintf(stderr, "Cannot read smbios file %s\n", buf);
> -- 
> 1.7.9
>
Andreas Färber Feb. 24, 2012, 9:15 a.m. UTC | #2 
Am 24.02.2012 01:36, schrieb David Gibson:
> Currently get_image_size(), used to find the size of files, returns an int.
> But for modern systems, int may be only 32-bit and we can have files
> larger than that.
> 
> This patch, therefore, changes the return type of get_image_size() to off_t
> (the same as the return type from lseek() itself).  It also audits all the
> callers of get_image_size() to make sure they process the new unsigned
> return type correctly.

It's a size, so why off_t and not size_t?

Andreas

> 
> This leaves load_image_targphys() with a limited return type, but one thing
> at a time (that function has far more callers to be audited, so it will
> take longer to fix).
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
David Gibson Feb. 24, 2012, 10:08 p.m. UTC | #3 
On Fri, Feb 24, 2012 at 10:15:51AM +0100, Andreas Färber wrote:
> Am 24.02.2012 01:36, schrieb David Gibson:
> > Currently get_image_size(), used to find the size of files, returns an int.
> > But for modern systems, int may be only 32-bit and we can have files
> > larger than that.
> > 
> > This patch, therefore, changes the return type of get_image_size() to off_t
> > (the same as the return type from lseek() itself).  It also audits all the
> > callers of get_image_size() to make sure they process the new unsigned
> > return type correctly.
> 
> It's a size, so why off_t and not size_t?

Because it's a file size, rather than an in-memory size.  Or more
specifically, off_t is the return type of lseek() which is where the
information is coming from in the first place.
Markus Armbruster Feb. 27, 2012, 8:21 a.m. UTC | #4 
David Gibson <david@gibson.dropbear.id.au> writes:

> Currently get_image_size(), used to find the size of files, returns an int.
> But for modern systems, int may be only 32-bit and we can have files
> larger than that.
>
> This patch, therefore, changes the return type of get_image_size() to off_t
> (the same as the return type from lseek() itself).  It also audits all the
> callers of get_image_size() to make sure they process the new unsigned
> return type correctly.
>
> This leaves load_image_targphys() with a limited return type, but one thing
> at a time (that function has far more callers to be audited, so it will
> take longer to fix).

I'm afraid this replaces the single, well-known integer overflow in
get_image_size()'s conversion of lseek() value to int by many unknown
overflows in get_image_size()'s users.  One example below.  Didn't look
for more.

If you need a wider get_image_size(), please make sure its users are
prepared for it!

Is the any use for image sizes exceeding size_t?  Arent such images
impossible to load?

[...]
> diff --git a/hw/pc.c b/hw/pc.c
> index b9f4bc7..cb41955 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -672,7 +672,8 @@ static void load_linux(void *fw_cfg,
>                         target_phys_addr_t max_ram_size)
>  {
>      uint16_t protocol;
> -    int setup_size, kernel_size, initrd_size = 0, cmdline_size;
> +    int setup_size, kernel_size, cmdline_size;
> +    off_t initrd_size = 0;
>      uint32_t initrd_max;
>      uint8_t header[8192], *setup, *kernel, *initrd_data;
>      target_phys_addr_t real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
> @@ -795,7 +796,7 @@ static void load_linux(void *fw_cfg,
>  	}
>  
>  	initrd_size = get_image_size(initrd_filename);
> -        if (initrd_size < 0) {
> +        if (initrd_size == -1) {

Needless churn.

>              fprintf(stderr, "qemu: error reading initrd %s\n",
>                      initrd_filename);
>              exit(1);
           }

           initrd_addr = (initrd_max-initrd_size) & ~4095;

           initrd_data = g_malloc(initrd_size);

Integer overflow in conversion from off_t initrd_size to the argument
type size_t[*].

           load_image(initrd_filename, initrd_data);

You could try replacing load_image() by a function that returns a
pointer to the malloced image contents.

[...]


[*] Technically some glib-defined type, because they're into making up
their own "portable" types for everything.
David Gibson Feb. 27, 2012, 8:27 a.m. UTC | #5 
On Mon, Feb 27, 2012 at 09:21:25AM +0100, Markus Armbruster wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > Currently get_image_size(), used to find the size of files, returns an int.
> > But for modern systems, int may be only 32-bit and we can have files
> > larger than that.
> >
> > This patch, therefore, changes the return type of get_image_size() to off_t
> > (the same as the return type from lseek() itself).  It also audits all the
> > callers of get_image_size() to make sure they process the new unsigned
> > return type correctly.
> >
> > This leaves load_image_targphys() with a limited return type, but one thing
> > at a time (that function has far more callers to be audited, so it will
> > take longer to fix).
> 
> I'm afraid this replaces the single, well-known integer overflow in
> get_image_size()'s conversion of lseek() value to int by many unknown
> overflows in get_image_size()'s users.  One example below.  Didn't look
> for more.
> 
> If you need a wider get_image_size(), please make sure its users are
> prepared for it!

Actually, I have no such need at all, but when I fixed another bug in
loader.c, someone whinged about me not changing get_image_size(), so
here it is.

> Is the any use for image sizes exceeding size_t?  Arent such images
> impossible to load?

Well, possibly not.

> 
> [...]
> > diff --git a/hw/pc.c b/hw/pc.c
> > index b9f4bc7..cb41955 100644
> > --- a/hw/pc.c
> > +++ b/hw/pc.c
> > @@ -672,7 +672,8 @@ static void load_linux(void *fw_cfg,
> >                         target_phys_addr_t max_ram_size)
> >  {
> >      uint16_t protocol;
> > -    int setup_size, kernel_size, initrd_size = 0, cmdline_size;
> > +    int setup_size, kernel_size, cmdline_size;
> > +    off_t initrd_size = 0;
> >      uint32_t initrd_max;
> >      uint8_t header[8192], *setup, *kernel, *initrd_data;
> >      target_phys_addr_t real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
> > @@ -795,7 +796,7 @@ static void load_linux(void *fw_cfg,
> >  	}
> >  
> >  	initrd_size = get_image_size(initrd_filename);
> > -        if (initrd_size < 0) {
> > +        if (initrd_size == -1) {
> 
> Needless churn.

No, it's not.  Now that initrd_size is unsigned initrd_size < 0 would
return false always (and give a "comparison is always false due to
limited range of data type" warning).

> 
> >              fprintf(stderr, "qemu: error reading initrd %s\n",
> >                      initrd_filename);
> >              exit(1);
>            }
> 
>            initrd_addr = (initrd_max-initrd_size) & ~4095;
> 
>            initrd_data = g_malloc(initrd_size);
> 
> Integer overflow in conversion from off_t initrd_size to the argument
> type size_t[*].

Hm, true.

Ok, well, I give up.  Someone who actually needs it can fix it.
Markus Armbruster Feb. 27, 2012, 9:31 a.m. UTC | #6 
David Gibson <david@gibson.dropbear.id.au> writes:

> On Mon, Feb 27, 2012 at 09:21:25AM +0100, Markus Armbruster wrote:
>> David Gibson <david@gibson.dropbear.id.au> writes:
>> 
>> > Currently get_image_size(), used to find the size of files, returns an int.
>> > But for modern systems, int may be only 32-bit and we can have files
>> > larger than that.
>> >
>> > This patch, therefore, changes the return type of get_image_size() to off_t
>> > (the same as the return type from lseek() itself).  It also audits all the
>> > callers of get_image_size() to make sure they process the new unsigned
>> > return type correctly.
>> >
>> > This leaves load_image_targphys() with a limited return type, but one thing
>> > at a time (that function has far more callers to be audited, so it will
>> > take longer to fix).
>> 
>> I'm afraid this replaces the single, well-known integer overflow in
>> get_image_size()'s conversion of lseek() value to int by many unknown
>> overflows in get_image_size()'s users.  One example below.  Didn't look
>> for more.
>> 
>> If you need a wider get_image_size(), please make sure its users are
>> prepared for it!
>
> Actually, I have no such need at all, but when I fixed another bug in
> loader.c, someone whinged about me not changing get_image_size(), so
> here it is.

Heh.

>> Is the any use for image sizes exceeding size_t?  Arent such images
>> impossible to load?
>
> Well, possibly not.
>
>> 
>> [...]
>> > diff --git a/hw/pc.c b/hw/pc.c
>> > index b9f4bc7..cb41955 100644
>> > --- a/hw/pc.c
>> > +++ b/hw/pc.c
>> > @@ -672,7 +672,8 @@ static void load_linux(void *fw_cfg,
>> >                         target_phys_addr_t max_ram_size)
>> >  {
>> >      uint16_t protocol;
>> > -    int setup_size, kernel_size, initrd_size = 0, cmdline_size;
>> > +    int setup_size, kernel_size, cmdline_size;
>> > +    off_t initrd_size = 0;
>> >      uint32_t initrd_max;
>> >      uint8_t header[8192], *setup, *kernel, *initrd_data;
>> >      target_phys_addr_t real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
>> > @@ -795,7 +796,7 @@ static void load_linux(void *fw_cfg,
>> >  	}
>> >  
>> >  	initrd_size = get_image_size(initrd_filename);
>> > -        if (initrd_size < 0) {
>> > +        if (initrd_size == -1) {
>> 
>> Needless churn.
>
> No, it's not.  Now that initrd_size is unsigned initrd_size < 0 would
> return false always (and give a "comparison is always false due to
> limited range of data type" warning).

off_t is signed:
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html

>> >              fprintf(stderr, "qemu: error reading initrd %s\n",
>> >                      initrd_filename);
>> >              exit(1);
>>            }
>> 
>>            initrd_addr = (initrd_max-initrd_size) & ~4095;
>> 
>>            initrd_data = g_malloc(initrd_size);
>> 
>> Integer overflow in conversion from off_t initrd_size to the argument
>> type size_t[*].
>
> Hm, true.
>
> Ok, well, I give up.  Someone who actually needs it can fix it.

Pity.  Thanks anyway!
diff mbox

Patch

diff --git a/device_tree.c b/device_tree.c
index 86a694c..5817b2d 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -27,14 +27,14 @@ 
 
 void *load_device_tree(const char *filename_path, int *sizep)
 {
-    int dt_size;
+    off_t dt_size;
     int dt_file_load_size;
     int ret;
     void *fdt = NULL;
 
     *sizep = 0;
     dt_size = get_image_size(filename_path);
-    if (dt_size < 0) {
+    if (dt_size == -1) {
         printf("Unable to get size of device tree file '%s'\n",
             filename_path);
         goto fail;
diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c
index ea0fd95..01ec9c7 100644
--- a/hw/alpha_dp264.c
+++ b/hw/alpha_dp264.c
@@ -144,10 +144,11 @@  static void clipper_init(ram_addr_t ram_size,
         }
 
         if (initrd_filename) {
-            long initrd_base, initrd_size;
+            long initrd_base;
+            off_t initrd_size;
 
             initrd_size = get_image_size(initrd_filename);
-            if (initrd_size < 0) {
+            if (initrd_size != -1) {
                 hw_error("could not load initial ram disk '%s'\n",
                          initrd_filename);
                 exit(1);
diff --git a/hw/highbank.c b/hw/highbank.c
index 489c00e..f4de4c6 100644
--- a/hw/highbank.c
+++ b/hw/highbank.c
@@ -235,7 +235,7 @@  static void highbank_init(ram_addr_t ram_size,
     if (bios_name != NULL) {
         sysboot_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
         if (sysboot_filename != NULL) {
-            uint32_t filesize = get_image_size(sysboot_filename);
+            off_t filesize = get_image_size(sysboot_filename);
             if (load_image_targphys("sysram.bin", 0xfff88000, filesize) < 0) {
                 hw_error("Unable to load %s\n", bios_name);
             }
diff --git a/hw/leon3.c b/hw/leon3.c
index 71d79a6..f7d4860 100644
--- a/hw/leon3.c
+++ b/hw/leon3.c
@@ -108,7 +108,7 @@  static void leon3_generic_hw_init(ram_addr_t  ram_size,
     int         ret;
     char       *filename;
     qemu_irq   *cpu_irqs = NULL;
-    int         bios_size;
+    off_t       bios_size;
     int         prom_size;
     ResetData  *reset_info;
 
@@ -168,7 +168,7 @@  static void leon3_generic_hw_init(ram_addr_t  ram_size,
         exit(1);
     }
 
-    if (bios_size > 0) {
+    if (bios_size != -1) {
         ret = load_image_targphys(filename, 0x00000000, bios_size);
         if (ret < 0 || ret > prom_size) {
             fprintf(stderr, "qemu: could not load prom '%s'\n", filename);
@@ -191,7 +191,7 @@  static void leon3_generic_hw_init(ram_addr_t  ram_size,
                     kernel_filename);
             exit(1);
         }
-        if (bios_size <= 0) {
+        if (bios_size == 0 || bios_size == -1) {
             /* If there is no bios/monitor, start the application.  */
             env->pc = entry;
             env->npc = entry + 4;
diff --git a/hw/loader.c b/hw/loader.c
index 415cdce..372f6f8 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -57,12 +57,14 @@ 
 static int roms_loaded;
 
 /* return the size or -1 if error */
-int get_image_size(const char *filename)
+off_t get_image_size(const char *filename)
 {
-    int fd, size;
+    int fd;
+    off_t size;
+
     fd = open(filename, O_RDONLY | O_BINARY);
     if (fd < 0)
-        return -1;
+        return (off_t)-1;
     size = lseek(fd, 0, SEEK_END);
     close(fd);
     return size;
@@ -105,13 +107,13 @@  ssize_t read_targphys(const char *name,
 int load_image_targphys(const char *filename,
 			target_phys_addr_t addr, int max_sz)
 {
-    int size;
+    off_t size;
 
     size = get_image_size(filename);
     if (size > max_sz) {
         return -1;
     }
-    if (size > 0) {
+    if (size != -1) {
         rom_add_file_fixed(filename, addr, -1);
     }
     return size;
diff --git a/hw/loader.h b/hw/loader.h
index fbcaba9..1fbdbcf 100644
--- a/hw/loader.h
+++ b/hw/loader.h
@@ -2,7 +2,7 @@ 
 #define LOADER_H
 
 /* loader.c */
-int get_image_size(const char *filename);
+off_t get_image_size(const char *filename);
 int load_image(const char *filename, uint8_t *addr); /* deprecated */
 int load_image_targphys(const char *filename, target_phys_addr_t, int max_sz);
 int load_elf(const char *filename, uint64_t (*translate_fn)(void *, uint64_t),
diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c
index e3ba9dd..1e7b49a 100644
--- a/hw/mips_fulong2e.c
+++ b/hw/mips_fulong2e.c
@@ -107,7 +107,7 @@  static int64_t load_kernel (CPUState *env)
 {
     int64_t kernel_entry, kernel_low, kernel_high;
     int index = 0;
-    long initrd_size;
+    off_t initrd_size;
     ram_addr_t initrd_offset;
     uint32_t *prom_buf;
     long prom_size;
@@ -125,7 +125,7 @@  static int64_t load_kernel (CPUState *env)
     initrd_offset = 0;
     if (loaderparams.initrd_filename) {
         initrd_size = get_image_size (loaderparams.initrd_filename);
-        if (initrd_size > 0) {
+        if (initrd_size != -1) {
             initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) & TARGET_PAGE_MASK;
             if (initrd_offset + initrd_size > ram_size) {
                 fprintf(stderr,
@@ -136,7 +136,7 @@  static int64_t load_kernel (CPUState *env)
             initrd_size = load_image_targphys(loaderparams.initrd_filename,
                                      initrd_offset, ram_size - initrd_offset);
         }
-        if (initrd_size == (target_ulong) -1) {
+        if (initrd_size == -1) {
             fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
                     loaderparams.initrd_filename);
             exit(1);
@@ -148,10 +148,10 @@  static int64_t load_kernel (CPUState *env)
     prom_buf = g_malloc(prom_size);
 
     prom_set(prom_buf, index++, "%s", loaderparams.kernel_filename);
-    if (initrd_size > 0) {
+    if (initrd_size != -1) {
         prom_set(prom_buf, index++, "rd_start=0x%" PRIx64 " rd_size=%li %s",
-                 cpu_mips_phys_to_kseg0(NULL, initrd_offset), initrd_size,
-                 loaderparams.kernel_cmdline);
+                 cpu_mips_phys_to_kseg0(NULL, initrd_offset),
+                 (long)initrd_size, loaderparams.kernel_cmdline);
     } else {
         prom_set(prom_buf, index++, "%s", loaderparams.kernel_cmdline);
     }
diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 86a5fbb..50e800a 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -667,7 +667,7 @@  static void GCC_FMT_ATTR(3, 4) prom_set(uint32_t* prom_buf, int index,
 static int64_t load_kernel (void)
 {
     int64_t kernel_entry, kernel_high;
-    long initrd_size;
+    off_t initrd_size;
     ram_addr_t initrd_offset;
     int big_endian;
     uint32_t *prom_buf;
@@ -693,7 +693,7 @@  static int64_t load_kernel (void)
     initrd_offset = 0;
     if (loaderparams.initrd_filename) {
         initrd_size = get_image_size (loaderparams.initrd_filename);
-        if (initrd_size > 0) {
+        if (initrd_size != -1) {
             initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) & TARGET_PAGE_MASK;
             if (initrd_offset + initrd_size > ram_size) {
                 fprintf(stderr,
@@ -705,7 +705,7 @@  static int64_t load_kernel (void)
                                               initrd_offset,
                                               ram_size - initrd_offset);
         }
-        if (initrd_size == (target_ulong) -1) {
+        if (initrd_size == -1) {
             fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
                     loaderparams.initrd_filename);
             exit(1);
@@ -717,10 +717,10 @@  static int64_t load_kernel (void)
     prom_buf = g_malloc(prom_size);
 
     prom_set(prom_buf, prom_index++, "%s", loaderparams.kernel_filename);
-    if (initrd_size > 0) {
+    if (initrd_size != -1) {
         prom_set(prom_buf, prom_index++, "rd_start=0x%" PRIx64 " rd_size=%li %s",
-                 cpu_mips_phys_to_kseg0(NULL, initrd_offset), initrd_size,
-                 loaderparams.kernel_cmdline);
+                 cpu_mips_phys_to_kseg0(NULL, initrd_offset),
+                 (long)initrd_size, loaderparams.kernel_cmdline);
     } else {
         prom_set(prom_buf, prom_index++, "%s", loaderparams.kernel_cmdline);
     }
diff --git a/hw/mips_mipssim.c b/hw/mips_mipssim.c
index 76c95b2..89de6ca 100644
--- a/hw/mips_mipssim.c
+++ b/hw/mips_mipssim.c
@@ -54,7 +54,7 @@  static int64_t load_kernel(void)
 {
     int64_t entry, kernel_high;
     long kernel_size;
-    long initrd_size;
+    off_t initrd_size;
     ram_addr_t initrd_offset;
     int big_endian;
 
@@ -82,7 +82,7 @@  static int64_t load_kernel(void)
     initrd_offset = 0;
     if (loaderparams.initrd_filename) {
         initrd_size = get_image_size (loaderparams.initrd_filename);
-        if (initrd_size > 0) {
+        if (initrd_size != -1) {
             initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) & TARGET_PAGE_MASK;
             if (initrd_offset + initrd_size > loaderparams.ram_size) {
                 fprintf(stderr,
@@ -93,7 +93,7 @@  static int64_t load_kernel(void)
             initrd_size = load_image_targphys(loaderparams.initrd_filename,
                 initrd_offset, loaderparams.ram_size - initrd_offset);
         }
-        if (initrd_size == (target_ulong) -1) {
+        if (initrd_size == -1) {
             fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
                     loaderparams.initrd_filename);
             exit(1);
diff --git a/hw/mips_r4k.c b/hw/mips_r4k.c
index 83401f0..067a793 100644
--- a/hw/mips_r4k.c
+++ b/hw/mips_r4k.c
@@ -72,7 +72,8 @@  typedef struct ResetData {
 static int64_t load_kernel(void)
 {
     int64_t entry, kernel_high;
-    long kernel_size, initrd_size, params_size;
+    long kernel_size, params_size;
+    off_t initrd_size;
     ram_addr_t initrd_offset;
     uint32_t *params_buf;
     int big_endian;
@@ -100,7 +101,7 @@  static int64_t load_kernel(void)
     initrd_offset = 0;
     if (loaderparams.initrd_filename) {
         initrd_size = get_image_size (loaderparams.initrd_filename);
-        if (initrd_size > 0) {
+        if (initrd_size != -1) {
             initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) & TARGET_PAGE_MASK;
             if (initrd_offset + initrd_size > ram_size) {
                 fprintf(stderr,
@@ -112,7 +113,7 @@  static int64_t load_kernel(void)
                                               initrd_offset,
                                               ram_size - initrd_offset);
         }
-        if (initrd_size == (target_ulong) -1) {
+        if (initrd_size == -1) {
             fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
                     loaderparams.initrd_filename);
             exit(1);
@@ -126,10 +127,10 @@  static int64_t load_kernel(void)
     params_buf[0] = tswap32(ram_size);
     params_buf[1] = tswap32(0x12345678);
 
-    if (initrd_size > 0) {
+    if (initrd_size != -1) {
         snprintf((char *)params_buf + 8, 256, "rd_start=0x%" PRIx64 " rd_size=%li %s",
                  cpu_mips_phys_to_kseg0(NULL, initrd_offset),
-                 initrd_size, loaderparams.kernel_cmdline);
+                 (long)initrd_size, loaderparams.kernel_cmdline);
     } else {
         snprintf((char *)params_buf + 8, 256, "%s", loaderparams.kernel_cmdline);
     }
@@ -161,7 +162,7 @@  void mips_r4k_init (ram_addr_t ram_size,
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *bios;
     MemoryRegion *iomem = g_new(MemoryRegion, 1);
-    int bios_size;
+    off_t bios_size;
     CPUState *env;
     ResetData *reset_info;
     int i;
@@ -214,14 +215,14 @@  void mips_r4k_init (ram_addr_t ram_size,
     if (filename) {
         bios_size = get_image_size(filename);
     } else {
-        bios_size = -1;
+        bios_size = (off_t)-1;
     }
 #ifdef TARGET_WORDS_BIGENDIAN
     be = 1;
 #else
     be = 0;
 #endif
-    if ((bios_size > 0) && (bios_size <= BIOS_SIZE)) {
+    if ((bios_size != -1) && (bios_size <= BIOS_SIZE)) {
         bios = g_new(MemoryRegion, 1);
         memory_region_init_ram(bios, "mips_r4k.bios", BIOS_SIZE);
         vmstate_register_ram_global(bios);
diff --git a/hw/multiboot.c b/hw/multiboot.c
index b4484a3..37e8af5 100644
--- a/hw/multiboot.c
+++ b/hw/multiboot.c
@@ -262,7 +262,7 @@  int load_multiboot(void *fw_cfg,
 
         do {
             char *next_space;
-            int mb_mod_length;
+            off_t mb_mod_length;
             uint32_t offs = mbs.mb_buf_size;
 
             next_initrd = (char *)get_opt_value(NULL, 0, initrd_filename);
@@ -275,7 +275,7 @@  int load_multiboot(void *fw_cfg,
                 *next_space = '\0';
             mb_debug("multiboot loading module: %s\n", initrd_filename);
             mb_mod_length = get_image_size(initrd_filename);
-            if (mb_mod_length < 0) {
+            if (mb_mod_length == -1) {
                 fprintf(stderr, "Failed to open file '%s'\n", initrd_filename);
                 exit(1);
             }
diff --git a/hw/palm.c b/hw/palm.c
index b1252ab..176ad4b 100644
--- a/hw/palm.c
+++ b/hw/palm.c
@@ -203,7 +203,8 @@  static void palmte_init(ram_addr_t ram_size,
     static uint32_t cs1val = 0x0000e1a0;
     static uint32_t cs2val = 0x0000e1a0;
     static uint32_t cs3val = 0xe1a0e1a0;
-    int rom_size, rom_loaded = 0;
+    int rom_loaded = 0;
+    off_t rom_size;
     DisplayState *ds = get_displaystate();
     MemoryRegion *flash = g_new(MemoryRegion, 1);
     MemoryRegion *cs = g_new(MemoryRegion, 4);
@@ -240,16 +241,16 @@  static void palmte_init(ram_addr_t ram_size,
     if (nb_option_roms) {
         rom_size = get_image_size(option_rom[0].name);
         if (rom_size > flash_size) {
-            fprintf(stderr, "%s: ROM image too big (%x > %x)\n",
-                            __FUNCTION__, rom_size, flash_size);
+            fprintf(stderr, "%s: ROM image too big (%lx > %x)\n",
+                    __FUNCTION__, (unsigned long)rom_size, flash_size);
             rom_size = 0;
         }
-        if (rom_size > 0) {
+        if (rom_size != -1) {
             rom_size = load_image_targphys(option_rom[0].name, OMAP_CS0_BASE,
                                            flash_size);
             rom_loaded = 1;
         }
-        if (rom_size < 0) {
+        if (rom_size == -1) {
             fprintf(stderr, "%s: error loading '%s'\n",
                             __FUNCTION__, option_rom[0].name);
         }
diff --git a/hw/pc.c b/hw/pc.c
index b9f4bc7..cb41955 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -672,7 +672,8 @@  static void load_linux(void *fw_cfg,
                        target_phys_addr_t max_ram_size)
 {
     uint16_t protocol;
-    int setup_size, kernel_size, initrd_size = 0, cmdline_size;
+    int setup_size, kernel_size, cmdline_size;
+    off_t initrd_size = 0;
     uint32_t initrd_max;
     uint8_t header[8192], *setup, *kernel, *initrd_data;
     target_phys_addr_t real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
@@ -795,7 +796,7 @@  static void load_linux(void *fw_cfg,
 	}
 
 	initrd_size = get_image_size(initrd_filename);
-        if (initrd_size < 0) {
+        if (initrd_size == -1) {
             fprintf(stderr, "qemu: error reading initrd %s\n",
                     initrd_filename);
             exit(1);
diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
index abf9004..7e2f715 100644
--- a/hw/pc_sysfw.c
+++ b/hw/pc_sysfw.c
@@ -132,7 +132,7 @@  static void old_pc_system_rom_init(MemoryRegion *rom_memory)
 {
     char *filename;
     MemoryRegion *bios, *isa_bios;
-    int bios_size, isa_bios_size;
+    off_t bios_size, isa_bios_size;
     int ret;
 
     /* BIOS load */
@@ -143,9 +143,9 @@  static void old_pc_system_rom_init(MemoryRegion *rom_memory)
     if (filename) {
         bios_size = get_image_size(filename);
     } else {
-        bios_size = -1;
+        bios_size = (off_t)-1;
     }
-    if (bios_size <= 0 ||
+    if ((bios_size == 0) || (bios_size == -1) ||
         (bios_size % 65536) != 0) {
         goto bios_error;
     }
diff --git a/hw/pci.c b/hw/pci.c
index bf046bf..5720a8c 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1672,7 +1672,7 @@  static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size)
 /* Add an option rom for the device */
 static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
 {
-    int size;
+    off_t size;
     char *path;
     void *ptr;
     char name[32];
@@ -1703,7 +1703,7 @@  static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
     }
 
     size = get_image_size(path);
-    if (size < 0) {
+    if (size == -1) {
         error_report("%s: failed to find romfile \"%s\"",
                      __FUNCTION__, pdev->romfile);
         g_free(path);
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index eb43fb5..c41e13c 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -489,7 +489,8 @@  static void ppc_prep_init (ram_addr_t ram_size,
 #if 0
     MemoryRegion *xcsr = g_new(MemoryRegion, 1);
 #endif
-    int linux_boot, i, nb_nics1, bios_size;
+    int linux_boot, i, nb_nics1;
+    off_t bios_size;
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *bios = g_new(MemoryRegion, 1);
     uint32_t kernel_base, initrd_base;
@@ -546,13 +547,13 @@  static void ppc_prep_init (ram_addr_t ram_size,
     } else {
         bios_size = -1;
     }
-    if (bios_size > 0 && bios_size <= BIOS_SIZE) {
+    if (bios_size != -1 && bios_size <= BIOS_SIZE) {
         target_phys_addr_t bios_addr;
         bios_size = (bios_size + 0xfff) & ~0xfff;
         bios_addr = (uint32_t)(-bios_size);
         bios_size = load_image_targphys(filename, bios_addr, bios_size);
     }
-    if (bios_size < 0 || bios_size > BIOS_SIZE) {
+    if (bios_size == -1 || bios_size > BIOS_SIZE) {
         hw_error("qemu: could not load PPC PREP bios '%s'\n", bios_name);
     }
     if (filename) {
diff --git a/hw/smbios.c b/hw/smbios.c
index c57237d..85fad0e 100644
--- a/hw/smbios.c
+++ b/hw/smbios.c
@@ -185,7 +185,7 @@  int smbios_entry_add(const char *t)
     if (get_param_value(buf, sizeof(buf), "file", t)) {
         struct smbios_structure_header *header;
         struct smbios_table *table;
-        int size = get_image_size(buf);
+        off_t size = get_image_size(buf);
 
         if (size == -1 || size < sizeof(struct smbios_structure_header)) {
             fprintf(stderr, "Cannot read smbios file %s\n", buf);