From 1fe1f4559e3efa5f4e30587212573c572b770369 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 17 Mar 2008 15:48:42 +0000 Subject: [PATCH] Remove most uses of phys_ram_base in hw/pc.c In this patch, I remove all the direct uses of phys_ram_base from hw/pc.c, except for those presently needed to construct the arguments to the vga init functions. This involved: * Getting rid of various additions and subtractions of phys_ram_base * Changing the types of the guest physical addresses in load_linux from uint8_t* to target_phys_addr_t * Replacing calls to memcpy and pstrcpy with cpu_physical_memory_write (and a new pstrcpy_targphys function) * Replacing most calls to fread with a new fread_targphys function * Deprecating load_image in favour of a new load_image_targphys * Removing (rather than fixing up) the unused function load_kernel I noticed that load_image doesn't take a buffer size argument - it just overwrites the destination buffer with file data, extending as long as the file happens to be. In most cases this is probably not an exploitable vulnerability, but it seems poor practice. Hence load_image_targphys's extra argument. --- hw/pc.c | 98 ++++++++++++++++++++++++-------------------------------- loader.c | 43 +++++++++++++++++++++++++ sysemu.h | 7 +++- 3 files changed, 91 insertions(+), 57 deletions(-) diff --git a/hw/pc.c b/hw/pc.c index c46537e9b..fbbc7a0f1 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -435,36 +435,6 @@ static void generate_bootsect(uint32_t gpr[8], uint16_t segs[6], uint16_t ip) bdrv_set_boot_sector(drives_table[hda].bdrv, bootsect, sizeof(bootsect)); } -static int load_kernel(const char *filename, uint8_t *addr, - uint8_t *real_addr) -{ - int fd, size; - int setup_sects; - - fd = open(filename, O_RDONLY | O_BINARY); - if (fd < 0) - return -1; - - /* load 16 bit code */ - if (qemu_read_ok(fd, real_addr, 512) < 0) - goto fail; - setup_sects = real_addr[0x1F1]; - if (!setup_sects) - setup_sects = 4; - if (qemu_read_ok(fd, real_addr + 512, setup_sects * 512) < 0) - goto fail; - - /* load 32 bit code */ - size = qemu_read(fd, addr, 16 * 1024 * 1024); - if (size < 0) - goto fail; - close(fd); - return size; - fail: - close(fd); - return -1; -} - static long get_file_size(FILE *f) { long where, size; @@ -479,6 +449,22 @@ static long get_file_size(FILE *f) return size; } +static void pstrcpy_targphys(target_phys_addr_t dest, int buf_size, + const char *source) +{ + static const char nul_byte; + const char *nulp; + + if (buf_size <= 0) return; + nulp = memchr(source, 0, buf_size); + if (nulp) { + cpu_physical_memory_write(dest, source, (nulp - source) + 1); + } else { + cpu_physical_memory_write(dest, source, buf_size - 1); + cpu_physical_memory_write(dest, &nul_byte, 1); + } +} + static void load_linux(const char *kernel_filename, const char *initrd_filename, const char *kernel_cmdline) @@ -490,7 +476,7 @@ static void load_linux(const char *kernel_filename, int setup_size, kernel_size, initrd_size, cmdline_size; uint32_t initrd_max; uint8_t header[1024]; - uint8_t *real_addr, *prot_addr, *cmdline_addr, *initrd_addr; + target_phys_addr_t real_addr, prot_addr, cmdline_addr, initrd_addr; FILE *f, *fi; /* Align to 16 bytes as a paranoia measure */ @@ -516,19 +502,19 @@ static void load_linux(const char *kernel_filename, if (protocol < 0x200 || !(header[0x211] & 0x01)) { /* Low kernel */ - real_addr = phys_ram_base + 0x90000; - cmdline_addr = phys_ram_base + 0x9a000 - cmdline_size; - prot_addr = phys_ram_base + 0x10000; + real_addr = 0x90000; + cmdline_addr = 0x9a000 - cmdline_size; + prot_addr = 0x10000; } else if (protocol < 0x202) { /* High but ancient kernel */ - real_addr = phys_ram_base + 0x90000; - cmdline_addr = phys_ram_base + 0x9a000 - cmdline_size; - prot_addr = phys_ram_base + 0x100000; + real_addr = 0x90000; + cmdline_addr = 0x9a000 - cmdline_size; + prot_addr = 0x100000; } else { /* High and recent kernel */ - real_addr = phys_ram_base + 0x10000; - cmdline_addr = phys_ram_base + 0x20000; - prot_addr = phys_ram_base + 0x100000; + real_addr = 0x10000; + cmdline_addr = 0x20000; + prot_addr = 0x100000; } #if 0 @@ -536,9 +522,9 @@ static void load_linux(const char *kernel_filename, "qemu: real_addr = %#zx\n" "qemu: cmdline_addr = %#zx\n" "qemu: prot_addr = %#zx\n", - real_addr-phys_ram_base, - cmdline_addr-phys_ram_base, - prot_addr-phys_ram_base); + real_addr, + cmdline_addr, + prot_addr); #endif /* highest address for loading the initrd */ @@ -551,10 +537,10 @@ static void load_linux(const char *kernel_filename, initrd_max = ram_size-ACPI_DATA_SIZE-1; /* kernel command line */ - pstrcpy((char*)cmdline_addr, 4096, kernel_cmdline); + pstrcpy_targphys(cmdline_addr, 4096, kernel_cmdline); if (protocol >= 0x202) { - stl_p(header+0x228, cmdline_addr-phys_ram_base); + stl_p(header+0x228, cmdline_addr); } else { stw_p(header+0x20, 0xA33F); stw_p(header+0x22, cmdline_addr-real_addr); @@ -588,24 +574,24 @@ static void load_linux(const char *kernel_filename, } initrd_size = get_file_size(fi); - initrd_addr = phys_ram_base + ((initrd_max-initrd_size) & ~4095); + initrd_addr = ((initrd_max-initrd_size) & ~4095); fprintf(stderr, "qemu: loading initrd (%#x bytes) at %#zx\n", - initrd_size, initrd_addr-phys_ram_base); + initrd_size, initrd_addr); - if (fread(initrd_addr, 1, initrd_size, fi) != initrd_size) { + if (!fread_targphys_ok(initrd_addr, initrd_size, fi)) { fprintf(stderr, "qemu: read error on initial ram disk '%s'\n", initrd_filename); exit(1); } fclose(fi); - stl_p(header+0x218, initrd_addr-phys_ram_base); + stl_p(header+0x218, initrd_addr); stl_p(header+0x21c, initrd_size); } /* store the finalized header and load the rest of the kernel */ - memcpy(real_addr, header, 1024); + cpu_physical_memory_write(real_addr, header, 1024); setup_size = header[0x1f1]; if (setup_size == 0) @@ -614,8 +600,8 @@ static void load_linux(const char *kernel_filename, setup_size = (setup_size+1)*512; kernel_size -= setup_size; /* Size of protected-mode code */ - if (fread(real_addr+1024, 1, setup_size-1024, f) != setup_size-1024 || - fread(prot_addr, 1, kernel_size, f) != kernel_size) { + if (!fread_targphys_ok(real_addr+1024, setup_size-1024, f) || + !fread_targphys_ok(prot_addr, kernel_size, f)) { fprintf(stderr, "qemu: read error on kernel '%s'\n", kernel_filename); exit(1); @@ -623,7 +609,7 @@ static void load_linux(const char *kernel_filename, fclose(f); /* generate bootsector to set up the initial register state */ - real_seg = (real_addr-phys_ram_base) >> 4; + real_seg = real_addr >> 4; seg[0] = seg[2] = seg[3] = seg[4] = seg[4] = real_seg; seg[1] = real_seg+0x20; /* CS */ memset(gpr, 0, sizeof gpr); @@ -764,7 +750,7 @@ static void pc_init1(int ram_size, int vga_ram_size, goto bios_error; } bios_offset = qemu_ram_alloc(bios_size); - ret = load_image(buf, phys_ram_base + bios_offset); + ret = load_image_targphys(buf, bios_offset, bios_size); if (ret != bios_size) { bios_error: fprintf(stderr, "qemu: could not load PC BIOS '%s'\n", buf); @@ -782,7 +768,7 @@ static void pc_init1(int ram_size, int vga_ram_size, goto vga_bios_error; vga_bios_offset = qemu_ram_alloc(65536); - ret = load_image(buf, phys_ram_base + vga_bios_offset); + ret = load_image_targphys(buf, vga_bios_offset, vga_bios_size); if (ret != vga_bios_size) { vga_bios_error: fprintf(stderr, "qemu: could not load VGA BIOS '%s'\n", buf); @@ -818,7 +804,7 @@ static void pc_init1(int ram_size, int vga_ram_size, if (size > (0x10000 - offset)) goto option_rom_error; option_rom_offset = qemu_ram_alloc(size); - ret = load_image(option_rom[i], phys_ram_base + option_rom_offset); + ret = load_image_targphys(option_rom[i], option_rom_offset, size); if (ret != size) { option_rom_error: fprintf(stderr, "Too many option ROMS\n"); diff --git a/loader.c b/loader.c index 813756e9c..5462d2405 100644 --- a/loader.c +++ b/loader.c @@ -39,6 +39,7 @@ int get_image_size(const char *filename) } /* return the size or -1 if error */ +/* deprecated, because caller does not specify buffer size! */ int load_image(const char *filename, uint8_t *addr) { int fd, size; @@ -55,6 +56,48 @@ int load_image(const char *filename, uint8_t *addr) return size; } +/* return the amount read, just like fread. 0 may mean error or eof */ +int fread_targphys(target_phys_addr_t dst_addr, size_t nbytes, FILE *f) +{ + unsigned char buf[4096]; + target_phys_addr_t dst_begin = dst_addr; + size_t want, did; + + while (nbytes) { + want = nbytes > sizeof(buf) ? sizeof(buf) : nbytes; + did = fread(buf, 1, want, f); + if (did != want) break; + + cpu_physical_memory_write(dst_addr, buf, did); + dst_addr += did; + nbytes -= did; + } + return dst_addr - dst_begin; +} + +/* returns 0 on error, 1 if ok */ +int fread_targphys_ok(target_phys_addr_t dst_addr, size_t nbytes, FILE *f) +{ + return fread_targphys(dst_addr, nbytes, f) == nbytes; +} + +/* return the size or -1 if error */ +int load_image_targphys(const char *filename, + target_phys_addr_t addr, int max_sz) +{ + FILE *f; + size_t got; + + f = fopen(filename, "rb"); + if (!f) return -1; + + got = fread_targphys(addr, max_sz, f); + if (ferror(f)) { fclose(f); return -1; } + fclose(f); + + return got; +} + /* A.OUT loader */ struct exec diff --git a/sysemu.h b/sysemu.h index 0f18e049e..ec91daf74 100644 --- a/sysemu.h +++ b/sysemu.h @@ -154,11 +154,16 @@ extern CharDriverState *parallel_hds[MAX_PARALLEL_PORTS]; #ifdef NEED_CPU_H /* loader.c */ int get_image_size(const char *filename); -int load_image(const char *filename, uint8_t *addr); +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, int64_t virt_to_phys_addend, uint64_t *pentry, uint64_t *lowaddr, uint64_t *highaddr); int load_aout(const char *filename, uint8_t *addr); int load_uboot(const char *filename, target_ulong *ep, int *is_linux); + +int fread_targphys(target_phys_addr_t dst_addr, size_t nbytes, FILE *f); +int fread_targphys_ok(target_phys_addr_t dst_addr, size_t nbytes, FILE *f); + #endif #ifdef HAS_AUDIO -- 2.39.5