David Gibson <da...@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.