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.

diff --git a/hw/pc.c b/hw/pc.c
index c46537e9b80a9a033ab7093296dd7cef395b7c7b..fbbc7a0f1852c4552087f76cf021daa5fd245bff 100644 (file)
--- 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 813756e9cf541434f7652119856d06a207e7e3f5..5462d24051832086d991bcdf50bdb809456ef47e 100644 (file)
--- 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 0f18e049e0fa0fb7984d4ec43cc65da6b8b3b659..ec91daf74a561c0fecf4b4944cf484a7b2051ce5 100644 (file)
--- 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