Josh Rickmar writes:
> On Fri, Mar 19, 2021 at 10:29:10AM -0400, Josh Rickmar wrote:
>> Here's an updated version of the patch I had originally posted to
>> bugs@ adding support for reading gzipped kernels (needed to boot amd64
>> bsd.rd without manually decompressing first), now that the support for
>> booting a kernel discovered on a ffs filesystem in the image file is
>> removed.
>>
>> I've kept the gzFile arguments named 'fp' to reduce the diff; let me
>> know if this should be changed to e.g. 'f' or 'gzf' so as to not
>> confuse it with FILE *.
I've tested the original diff and it works for me.
Anyone else?
>
> Small update: removed <stdio.h> from vmd.h since it's no longer
> needed, and add missing <stdio.h> to vioqcow2.c.
There are numerous includes that could be cleaned up. That's a separate
issue.
>
> diff a13de4d12a4c9ba0edc05aab2ad635f782449229 /usr/src
> blob - 132221fb960ae8a9184aaeb7b26669d7d715bdf1
> file + usr.sbin/vmd/Makefile
> --- usr.sbin/vmd/Makefile
> +++ usr.sbin/vmd/Makefile
> @@ -14,8 +14,8 @@ CFLAGS+= -Wmissing-declarations
> CFLAGS+= -Wshadow -Wpointer-arith -Wcast-qual
> CFLAGS+= -Wsign-compare
>
> -LDADD+= -lutil -lpthread -levent
> -DPADD+= ${LIBUTIL} ${LIBPTHREAD} ${LIBEVENT}
> +LDADD+= -lutil -lpthread -levent -lz
> +DPADD+= ${LIBUTIL} ${LIBPTHREAD} ${LIBEVENT} ${LIBZ}
>
> YFLAGS=
>
> blob - 43b79cf6762f77c761723c2189546e9a7fafd79f
> file + usr.sbin/vmd/loadfile.h
> --- usr.sbin/vmd/loadfile.h
> +++ usr.sbin/vmd/loadfile.h
> @@ -30,6 +30,8 @@
> * POSSIBILITY OF SUCH DAMAGE.
> */
>
> +#include <zlib.h>
> +
> /*
> * Array indices in the u_long position array
> */
> @@ -73,6 +75,6 @@
> #define PML2_PAGE 0x13000
> #define NPTE_PG (PAGE_SIZE / sizeof(uint64_t))
>
> -int loadfile_elf(FILE *, struct vm_create_params *, struct vcpu_reg_state *);
> +int loadfile_elf(gzFile, struct vm_create_params *, struct vcpu_reg_state *);
>
> -size_t mread(FILE *, paddr_t, size_t);
> +size_t mread(gzFile, paddr_t, size_t);
> blob - 8485ac59ccbc3459d37db1c6e2660b6862b11bd8
> file + usr.sbin/vmd/loadfile_elf.c
> --- usr.sbin/vmd/loadfile_elf.c
> +++ usr.sbin/vmd/loadfile_elf.c
> @@ -115,8 +115,8 @@ union {
>
> static void setsegment(struct mem_segment_descriptor *, uint32_t,
> size_t, int, int, int, int);
> -static int elf32_exec(FILE *, Elf32_Ehdr *, u_long *, int);
> -static int elf64_exec(FILE *, Elf64_Ehdr *, u_long *, int);
> +static int elf32_exec(gzFile, Elf32_Ehdr *, u_long *, int);
> +static int elf64_exec(gzFile, Elf64_Ehdr *, u_long *, int);
> static size_t create_bios_memmap(struct vm_create_params *, bios_memmap_t *);
> static uint32_t push_bootargs(bios_memmap_t *, size_t);
> static size_t push_stack(uint32_t, uint32_t);
> @@ -260,10 +260,11 @@ push_pt_64(void)
> *
> * Return values:
> * 0 if successful
> - * various error codes returned from read(2) or loadelf functions
> + * various error codes returned from gzread(3) or loadelf functions
> */
> int
> -loadfile_elf(FILE *fp, struct vm_create_params *vcp, struct vcpu_reg_state
> *vrs)
> +loadfile_elf(gzFile fp, struct vm_create_params *vcp,
> + struct vcpu_reg_state *vrs)
> {
> int r, is_i386 = 0;
> uint32_t bootargsz;
> @@ -271,7 +272,7 @@ loadfile_elf(FILE *fp, struct vm_create_params *vcp, s
> u_long marks[MARK_MAX];
> bios_memmap_t memmap[VMM_MAX_MEM_RANGES + 1];
>
> - if ((r = fread(&hdr, 1, sizeof(hdr), fp)) != sizeof(hdr))
> + if ((r = gzread(fp, &hdr, sizeof(hdr))) != sizeof(hdr))
> return 1;
>
> memset(&marks, 0, sizeof(marks));
> @@ -471,7 +472,7 @@ push_stack(uint32_t bootargsz, uint32_t end)
> * into the guest address space at paddr 'addr'.
> *
> * Parameters:
> - * fd: file descriptor of the kernel image file to read from.
> + * fp: kernel image file to read from.
> * addr: guest paddr_t to load to
> * sz: number of bytes to load
> *
> @@ -479,7 +480,7 @@ push_stack(uint32_t bootargsz, uint32_t end)
> * returns 'sz' if successful, or 0 otherwise.
> */
> size_t
> -mread(FILE *fp, paddr_t addr, size_t sz)
> +mread(gzFile fp, paddr_t addr, size_t sz)
> {
> size_t ct;
> size_t i, rd, osz;
> @@ -499,7 +500,7 @@ mread(FILE *fp, paddr_t addr, size_t sz)
> else
> ct = sz;
>
> - if (fread(buf, 1, ct, fp) != ct) {
> + if ((size_t)gzread(fp, buf, ct) != ct) {
> log_warn("%s: error %d in mread", __progname, errno);
> return (0);
> }
> @@ -523,7 +524,7 @@ mread(FILE *fp, paddr_t addr, size_t sz)
> else
> ct = PAGE_SIZE;
>
> - if (fread(buf, 1, ct, fp) != ct) {
> + if ((size_t)gzread(fp, buf, ct) != ct) {
> log_warn("%s: error %d in mread", __progname, errno);
> return (0);
> }
> @@ -628,13 +629,13 @@ mbcopy(void *src, paddr_t dst, int sz)
> /*
> * elf64_exec
> *
> - * Load the kernel indicated by 'fd' into the guest physical memory
> + * Load the kernel indicated by 'fp' into the guest physical memory
> * space, at the addresses defined in the ELF header.
> *
> * This function is used for 64 bit kernels.
> *
> * Parameters:
> - * fd: file descriptor of the kernel to load
> + * fp: kernel image file to load
> * elf: ELF header of the kernel
> * marks: array to store the offsets of various kernel structures
> * (start, bss, etc)
> @@ -646,7 +647,7 @@ mbcopy(void *src, paddr_t dst, int sz)
> * 1 if unsuccessful
> */
> static int
> -elf64_exec(FILE *fp, Elf64_Ehdr *elf, u_long *marks, int flags)
> +elf64_exec(gzFile fp, Elf64_Ehdr *elf, u_long *marks, int flags)
> {
> Elf64_Shdr *shp;
> Elf64_Phdr *phdr;
> @@ -661,12 +662,12 @@ elf64_exec(FILE *fp, Elf64_Ehdr *elf, u_long *marks, i
> sz = elf->e_phnum * sizeof(Elf64_Phdr);
> phdr = malloc(sz);
>
> - if (fseeko(fp, (off_t)elf->e_phoff, SEEK_SET) == -1) {
> + if (gzseek(fp, (off_t)elf->e_phoff, SEEK_SET) == -1) {
> free(phdr);
> return 1;
> }
>
> - if (fread(phdr, 1, sz, fp) != sz) {
> + if ((size_t)gzread(fp, phdr, sz) != sz) {
> free(phdr);
> return 1;
> }
> @@ -706,7 +707,7 @@ elf64_exec(FILE *fp, Elf64_Ehdr *elf, u_long *marks, i
> (IS_DATA(phdr[i]) && (flags & LOAD_DATA))) {
>
> /* Read in segment. */
> - if (fseeko(fp, (off_t)phdr[i].p_offset,
> + if (gzseek(fp, (off_t)phdr[i].p_offset,
> SEEK_SET) == -1) {
> free(phdr);
> return 1;
> @@ -751,14 +752,14 @@ elf64_exec(FILE *fp, Elf64_Ehdr *elf, u_long *marks, i
> maxp += sizeof(Elf64_Ehdr);
>
> if (flags & (LOAD_SYM | COUNT_SYM)) {
> - if (fseeko(fp, (off_t)elf->e_shoff, SEEK_SET) == -1) {
> - warn("lseek section headers");
> + if (gzseek(fp, (off_t)elf->e_shoff, SEEK_SET) == -1) {
> + warn("gzseek section headers");
> return 1;
> }
> sz = elf->e_shnum * sizeof(Elf64_Shdr);
> shp = malloc(sz);
>
> - if (fread(shp, 1, sz, fp) != sz) {
> + if ((size_t)gzread(fp, shp, sz) != sz) {
> free(shp);
> return 1;
> }
> @@ -768,13 +769,13 @@ elf64_exec(FILE *fp, Elf64_Ehdr *elf, u_long *marks, i
>
> size_t shstrsz = shp[elf->e_shstrndx].sh_size;
> char *shstr = malloc(shstrsz);
> - if (fseeko(fp, (off_t)shp[elf->e_shstrndx].sh_offset,
> + if (gzseek(fp, (off_t)shp[elf->e_shstrndx].sh_offset,
> SEEK_SET) == -1) {
> free(shstr);
> free(shp);
> return 1;
> }
> - if (fread(shstr, 1, shstrsz, fp) != shstrsz) {
> + if ((size_t)gzread(fp, shstr, shstrsz) != shstrsz) {
> free(shstr);
> free(shp);
> return 1;
> @@ -797,7 +798,7 @@ elf64_exec(FILE *fp, Elf64_Ehdr *elf, u_long *marks, i
> !strcmp(shstr + shp[i].sh_name, ".debug_line") ||
> !strcmp(shstr + shp[i].sh_name, ELF_CTF)) {
> if (havesyms && (flags & LOAD_SYM)) {
> - if (fseeko(fp, (off_t)shp[i].sh_offset,
> + if (gzseek(fp, (off_t)shp[i].sh_offset,
> SEEK_SET) == -1) {
> free(shstr);
> free(shp);
> @@ -850,13 +851,13 @@ elf64_exec(FILE *fp, Elf64_Ehdr *elf, u_long *marks, i
> /*
> * elf32_exec
> *
> - * Load the kernel indicated by 'fd' into the guest physical memory
> + * Load the kernel indicated by 'fp' into the guest physical memory
> * space, at the addresses defined in the ELF header.
> *
> * This function is used for 32 bit kernels.
> *
> * Parameters:
> - * fd: file descriptor of the kernel to load
> + * fp: kernel image file to load
> * elf: ELF header of the kernel
> * marks: array to store the offsets of various kernel structures
> * (start, bss, etc)
> @@ -868,7 +869,7 @@ elf64_exec(FILE *fp, Elf64_Ehdr *elf, u_long *marks, i
> * 1 if unsuccessful
> */
> static int
> -elf32_exec(FILE *fp, Elf32_Ehdr *elf, u_long *marks, int flags)
> +elf32_exec(gzFile fp, Elf32_Ehdr *elf, u_long *marks, int flags)
> {
> Elf32_Shdr *shp;
> Elf32_Phdr *phdr;
> @@ -883,12 +884,12 @@ elf32_exec(FILE *fp, Elf32_Ehdr *elf, u_long *marks, i
> sz = elf->e_phnum * sizeof(Elf32_Phdr);
> phdr = malloc(sz);
>
> - if (fseeko(fp, (off_t)elf->e_phoff, SEEK_SET) == -1) {
> + if (gzseek(fp, (off_t)elf->e_phoff, SEEK_SET) == -1) {
> free(phdr);
> return 1;
> }
>
> - if (fread(phdr, 1, sz, fp) != sz) {
> + if ((size_t)gzread(fp, phdr, sz) != sz) {
> free(phdr);
> return 1;
> }
> @@ -928,7 +929,7 @@ elf32_exec(FILE *fp, Elf32_Ehdr *elf, u_long *marks, i
> (IS_DATA(phdr[i]) && (flags & LOAD_DATA))) {
>
> /* Read in segment. */
> - if (fseeko(fp, (off_t)phdr[i].p_offset,
> + if (gzseek(fp, (off_t)phdr[i].p_offset,
> SEEK_SET) == -1) {
> free(phdr);
> return 1;
> @@ -973,14 +974,14 @@ elf32_exec(FILE *fp, Elf32_Ehdr *elf, u_long *marks, i
> maxp += sizeof(Elf32_Ehdr);
>
> if (flags & (LOAD_SYM | COUNT_SYM)) {
> - if (fseeko(fp, (off_t)elf->e_shoff, SEEK_SET) == -1) {
> + if (gzseek(fp, (off_t)elf->e_shoff, SEEK_SET) == -1) {
> warn("lseek section headers");
> return 1;
> }
> sz = elf->e_shnum * sizeof(Elf32_Shdr);
> shp = malloc(sz);
>
> - if (fread(shp, 1, sz, fp) != sz) {
> + if ((size_t)gzread(fp, shp, sz) != sz) {
> free(shp);
> return 1;
> }
> @@ -990,13 +991,13 @@ elf32_exec(FILE *fp, Elf32_Ehdr *elf, u_long *marks, i
>
> size_t shstrsz = shp[elf->e_shstrndx].sh_size;
> char *shstr = malloc(shstrsz);
> - if (fseeko(fp, (off_t)shp[elf->e_shstrndx].sh_offset,
> + if (gzseek(fp, (off_t)shp[elf->e_shstrndx].sh_offset,
> SEEK_SET) == -1) {
> free(shstr);
> free(shp);
> return 1;
> }
> - if (fread(shstr, 1, shstrsz, fp) != shstrsz) {
> + if ((size_t)gzread(fp, shstr, shstrsz) != shstrsz) {
> free(shstr);
> free(shp);
> return 1;
> @@ -1018,7 +1019,7 @@ elf32_exec(FILE *fp, Elf32_Ehdr *elf, u_long *marks, i
> shp[i].sh_type == SHT_STRTAB ||
> !strcmp(shstr + shp[i].sh_name, ".debug_line")) {
> if (havesyms && (flags & LOAD_SYM)) {
> - if (fseeko(fp, (off_t)shp[i].sh_offset,
> + if (gzseek(fp, (off_t)shp[i].sh_offset,
> SEEK_SET) == -1) {
> free(shstr);
> free(shp);
> blob - 34d0f116cc44d04a905a18214a8fb676d4a122a8
> file + usr.sbin/vmd/vioqcow2.c
> --- usr.sbin/vmd/vioqcow2.c
> +++ usr.sbin/vmd/vioqcow2.c
> @@ -22,6 +22,7 @@
> #include <machine/vmmvar.h>
> #include <dev/pci/pcireg.h>
>
> +#include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> blob - bf515eb3778b0bd79da29a52f47f1fa946985329
> file + usr.sbin/vmd/vm.c
> --- usr.sbin/vmd/vm.c
> +++ usr.sbin/vmd/vm.c
> @@ -21,6 +21,7 @@
> #include <sys/queue.h>
> #include <sys/wait.h>
> #include <sys/uio.h>
> +#include <sys/stat.h>
> #include <sys/socket.h>
> #include <sys/time.h>
> #include <sys/mman.h>
> @@ -84,7 +85,7 @@ void vcpu_exit_inout(struct vm_run_params *);
> int vcpu_exit_eptviolation(struct vm_run_params *);
> uint8_t vcpu_exit_pci(struct vm_run_params *);
> int vcpu_pic_intr(uint32_t, uint32_t, uint8_t);
> -int loadfile_bios(FILE *, struct vcpu_reg_state *);
> +int loadfile_bios(gzFile, off_t, struct vcpu_reg_state *);
> int send_vm(int, struct vm_create_params *);
> int dump_send_header(int);
> int dump_vmr(int , struct vm_mem_range *);
> @@ -213,6 +214,7 @@ static const struct vcpu_reg_state vcpu_init_flat16 =
> *
> * Parameters:
> * fp: file of a kernel file to load
> + * size: uncompressed size of the image
> * (out) vrs: register state to set on init for this kernel
> *
> * Return values:
> @@ -220,16 +222,15 @@ static const struct vcpu_reg_state vcpu_init_flat16 =
> * various error codes returned from read(2) or loadelf functions
> */
> int
> -loadfile_bios(FILE *fp, struct vcpu_reg_state *vrs)
> +loadfile_bios(gzFile fp, off_t size, struct vcpu_reg_state *vrs)
> {
> - off_t size, off;
> + off_t off;
>
> /* Set up a "flat 16 bit" register state for BIOS */
> memcpy(vrs, &vcpu_init_flat16, sizeof(*vrs));
>
> - /* Get the size of the BIOS image and seek to the beginning */
> - if (fseeko(fp, 0, SEEK_END) == -1 || (size = ftello(fp)) == -1 ||
> - fseeko(fp, 0, SEEK_SET) == -1)
> + /* Seek to the beginning of the BIOS image */
> + if (gzseek(fp, 0, SEEK_SET) == -1)
> return (-1);
>
> /* The BIOS image must end at 1M */
> @@ -277,9 +278,10 @@ start_vm(struct vmd_vm *vm, int fd)
> struct vcpu_reg_state vrs;
> int nicfds[VMM_MAX_NICS_PER_VM];
> int ret;
> - FILE *fp;
> + gzFile fp;
> size_t i;
> struct vm_rwregs_params vrp;
> + struct stat sb;
>
> /* Child */
> setproctitle("%s", vcp->vcp_name);
> @@ -331,7 +333,7 @@ start_vm(struct vmd_vm *vm, int fd)
> memcpy(&vrs, &vcpu_init_flat64, sizeof(vrs));
>
> /* Find and open kernel image */
> - if ((fp = fdopen(vm->vm_kernel, "r")) == NULL)
> + if ((fp = gzdopen(vm->vm_kernel, "r")) == NULL)
> fatalx("failed to open kernel - exiting");
>
> /* Load kernel image */
> @@ -339,16 +341,16 @@ start_vm(struct vmd_vm *vm, int fd)
>
> /*
> * Try BIOS as a fallback (only if it was provided as an image
> - * with vm->vm_kernel and not loaded from the disk)
> + * with vm->vm_kernel and the file is not compressed)
> */
> - if (ret && errno == ENOEXEC && vm->vm_kernel != -1)
> - ret = loadfile_bios(fp, &vrs);
> + if (ret && errno == ENOEXEC && vm->vm_kernel != -1 &&
> + gzdirect(fp) && (ret = fstat(vm->vm_kernel, &sb)) == 0)
> + ret = loadfile_bios(fp, sb.st_size, &vrs);
>
> if (ret)
> fatal("failed to load kernel or BIOS - exiting");
>
> - if (fp)
> - fclose(fp);
> + gzclose(fp);
> }
>
> if (vm->vm_kernel != -1)
> blob - a2d80eb2181bf45a6adb5a87774be9d541b9041d
> file + usr.sbin/vmd/vmd.h
> --- usr.sbin/vmd/vmd.h
> +++ usr.sbin/vmd/vmd.h
> @@ -28,7 +28,6 @@
> #include <netinet6/in6_var.h>
>
> #include <limits.h>
> -#include <stdio.h>
> #include <pthread.h>
>
> #include "proc.h"
> @@ -472,10 +471,6 @@ int config_getdisk(struct privsep *, struct imsg
> *);
> int config_getif(struct privsep *, struct imsg *);
> int config_getcdrom(struct privsep *, struct imsg *);
>
> -/* vmboot.c */
> -FILE *vmboot_open(int, int *, int, unsigned int, struct vmboot_params *);
> -void vmboot_close(FILE *, struct vmboot_params *);
> -
> /* parse.y */
> int parse_config(const char *);
> int cmdline_symset(char *);
