* [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed @ 2014-08-02 15:45 Richard W.M. Jones 2014-08-02 15:45 ` [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed kernel Richard W.M. Jones 0 siblings, 1 reply; 6+ messages in thread From: Richard W.M. Jones @ 2014-08-02 15:45 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell No change compared to v3. I just rebased the patch on top of current HEAD and ensured that it still works. Any comments at all on this approach? It's the last patch I need to make libguestfs work on aarch64 ... Rich. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed kernel. 2014-08-02 15:45 [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed Richard W.M. Jones @ 2014-08-02 15:45 ` Richard W.M. Jones 2014-08-03 23:05 ` Peter Crosthwaite 0 siblings, 1 reply; 6+ messages in thread From: Richard W.M. Jones @ 2014-08-02 15:45 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell On aarch64 it is the bootloader's job to uncompress the kernel. UEFI and u-boot bootloaders do this automatically when the kernel is gzip-compressed. However the qemu -kernel option does not do this. The following command does not work: qemu-system-aarch64 [...] -kernel /boot/vmlinuz because it tries to execute the gzip-compressed data. This commit lets gzip-compressed kernels be uncompressed transparently. Currently this is only done when emulating aarch64. Signed-off-by: Richard W.M. Jones <rjones@redhat.com> --- hw/arm/boot.c | 9 +++++++++ hw/core/loader.c | 43 +++++++++++++++++++++++++++++++++++++++++++ include/hw/loader.h | 1 + 3 files changed, 53 insertions(+) diff --git a/hw/arm/boot.c b/hw/arm/boot.c index 3d1f4a2..1d541db 100644 --- a/hw/arm/boot.c +++ b/hw/arm/boot.c @@ -444,6 +444,7 @@ static void do_cpu_reset(void *opaque) void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) { CPUState *cs = CPU(cpu); + int allow_compressed_kernels = 0; int kernel_size; int initrd_size; int is_linux = 0; @@ -465,6 +466,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) primary_loader = bootloader_aarch64; kernel_load_offset = KERNEL64_LOAD_ADDR; elf_machine = EM_AARCH64; + allow_compressed_kernels = 1; } else { primary_loader = bootloader; kernel_load_offset = KERNEL_LOAD_ADDR; @@ -510,6 +512,13 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) kernel_size = load_uimage(info->kernel_filename, &entry, NULL, &is_linux); } + /* On aarch64, it's the bootloader's job to uncompress the kernel. */ + if (allow_compressed_kernels && kernel_size < 0) { + entry = info->loader_start + kernel_load_offset; + kernel_size = load_image_gzipped(info->kernel_filename, entry, + info->ram_size - kernel_load_offset); + is_linux = 1; + } if (kernel_size < 0) { entry = info->loader_start + kernel_load_offset; kernel_size = load_image_targphys(info->kernel_filename, entry, diff --git a/hw/core/loader.c b/hw/core/loader.c index 2bf6b8f..1cbe733 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -577,6 +577,49 @@ int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz) return load_uboot_image(filename, NULL, &addr, NULL, IH_TYPE_RAMDISK); } +/* Load a gzip-compressed kernel (currently used by aarch64). */ +int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz) +{ + uint8_t *compressed_data = NULL; + uint8_t *data = NULL; + gsize len; + size_t max_bytes; + ssize_t bytes; + int ret = -1; + + if (!g_file_get_contents(filename, (char **) &compressed_data, &len, + NULL)) { + goto out; + } + + /* Is it a gzip-compressed file? */ + if (len < 2 || + compressed_data[0] != '\x1f' || + compressed_data[1] != '\x8b') { + goto out; + } + + max_bytes = UBOOT_MAX_GUNZIP_BYTES; + if (max_bytes > max_sz) { + max_bytes = max_sz; + } + data = g_malloc(max_bytes); + bytes = gunzip(data, max_bytes, compressed_data, len); + if (bytes < 0) { + fprintf(stderr, "%s: unable to decompress gzipped kernel file\n", + filename); + goto out; + } + + rom_add_blob_fixed(filename, data, bytes, addr); + ret = bytes; + + out: + g_free(compressed_data); + g_free(data); + return ret; +} + /* * Functions for reboot-persistent memory regions. * - used for vga bios and option roms. diff --git a/include/hw/loader.h b/include/hw/loader.h index 796cbf9..00c9117 100644 --- a/include/hw/loader.h +++ b/include/hw/loader.h @@ -15,6 +15,7 @@ int get_image_size(const char *filename); int load_image(const char *filename, uint8_t *addr); /* deprecated */ int load_image_targphys(const char *filename, hwaddr, uint64_t max_sz); +int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz); #define ELF_LOAD_FAILED -1 #define ELF_LOAD_NOT_ELF -2 -- 2.0.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed kernel. 2014-08-02 15:45 ` [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed kernel Richard W.M. Jones @ 2014-08-03 23:05 ` Peter Crosthwaite 2014-08-04 8:48 ` Richard W.M. Jones 0 siblings, 1 reply; 6+ messages in thread From: Peter Crosthwaite @ 2014-08-03 23:05 UTC (permalink / raw) To: Richard W.M. Jones; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers On Sun, Aug 3, 2014 at 1:45 AM, Richard W.M. Jones <rjones@redhat.com> wrote: > On aarch64 it is the bootloader's job to uncompress the kernel. UEFI > and u-boot bootloaders do this automatically when the kernel is > gzip-compressed. > > However the qemu -kernel option does not do this. The following > command does not work: > > qemu-system-aarch64 [...] -kernel /boot/vmlinuz > > because it tries to execute the gzip-compressed data. > > This commit lets gzip-compressed kernels be uncompressed > transparently. > > Currently this is only done when emulating aarch64. > > Signed-off-by: Richard W.M. Jones <rjones@redhat.com> > --- > hw/arm/boot.c | 9 +++++++++ > hw/core/loader.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > include/hw/loader.h | 1 + There's two patches here, one adding the generic gzip loader (which has wider review audience than just ARM people), and a second one adding the AArch64 usage. > 3 files changed, 53 insertions(+) > > diff --git a/hw/arm/boot.c b/hw/arm/boot.c > index 3d1f4a2..1d541db 100644 > --- a/hw/arm/boot.c > +++ b/hw/arm/boot.c > @@ -444,6 +444,7 @@ static void do_cpu_reset(void *opaque) > void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) > { > CPUState *cs = CPU(cpu); > + int allow_compressed_kernels = 0; > int kernel_size; > int initrd_size; > int is_linux = 0; > @@ -465,6 +466,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) > primary_loader = bootloader_aarch64; > kernel_load_offset = KERNEL64_LOAD_ADDR; > elf_machine = EM_AARCH64; > + allow_compressed_kernels = 1; > } else { > primary_loader = bootloader; > kernel_load_offset = KERNEL_LOAD_ADDR; > @@ -510,6 +512,13 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) > kernel_size = load_uimage(info->kernel_filename, &entry, NULL, > &is_linux); > } > + /* On aarch64, it's the bootloader's job to uncompress the kernel. */ > + if (allow_compressed_kernels && kernel_size < 0) { > + entry = info->loader_start + kernel_load_offset; > + kernel_size = load_image_gzipped(info->kernel_filename, entry, > + info->ram_size - kernel_load_offset); > + is_linux = 1; > + } > if (kernel_size < 0) { > entry = info->loader_start + kernel_load_offset; > kernel_size = load_image_targphys(info->kernel_filename, entry, > diff --git a/hw/core/loader.c b/hw/core/loader.c > index 2bf6b8f..1cbe733 100644 > --- a/hw/core/loader.c > +++ b/hw/core/loader.c > @@ -577,6 +577,49 @@ int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz) > return load_uboot_image(filename, NULL, &addr, NULL, IH_TYPE_RAMDISK); > } > > +/* Load a gzip-compressed kernel (currently used by aarch64). */ No need to refer to "aarch64" in generic code (this comment is likely to go stale). > +int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz) > +{ > + uint8_t *compressed_data = NULL; > + uint8_t *data = NULL; > + gsize len; > + size_t max_bytes; > + ssize_t bytes; > + int ret = -1; > + > + if (!g_file_get_contents(filename, (char **) &compressed_data, &len, > + NULL)) { > + goto out; > + } > + > + /* Is it a gzip-compressed file? */ > + if (len < 2 || > + compressed_data[0] != '\x1f' || > + compressed_data[1] != '\x8b') { > + goto out; > + } > + > + max_bytes = UBOOT_MAX_GUNZIP_BYTES; Why does u-boot's maximum size limit apply here? Regards, Peter > + if (max_bytes > max_sz) { > + max_bytes = max_sz; > + } > + data = g_malloc(max_bytes); > + bytes = gunzip(data, max_bytes, compressed_data, len); > + if (bytes < 0) { > + fprintf(stderr, "%s: unable to decompress gzipped kernel file\n", > + filename); > + goto out; > + } > + > + rom_add_blob_fixed(filename, data, bytes, addr); > + ret = bytes; > + > + out: > + g_free(compressed_data); > + g_free(data); > + return ret; > +} > + > /* > * Functions for reboot-persistent memory regions. > * - used for vga bios and option roms. > diff --git a/include/hw/loader.h b/include/hw/loader.h > index 796cbf9..00c9117 100644 > --- a/include/hw/loader.h > +++ b/include/hw/loader.h > @@ -15,6 +15,7 @@ int get_image_size(const char *filename); > int load_image(const char *filename, uint8_t *addr); /* deprecated */ > int load_image_targphys(const char *filename, hwaddr, > uint64_t max_sz); > +int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz); > > #define ELF_LOAD_FAILED -1 > #define ELF_LOAD_NOT_ELF -2 > -- > 2.0.1 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed kernel. 2014-08-03 23:05 ` Peter Crosthwaite @ 2014-08-04 8:48 ` Richard W.M. Jones 2014-08-04 8:52 ` Peter Maydell 2014-08-04 9:11 ` Peter Crosthwaite 0 siblings, 2 replies; 6+ messages in thread From: Richard W.M. Jones @ 2014-08-04 8:48 UTC (permalink / raw) To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers On Mon, Aug 04, 2014 at 09:05:39AM +1000, Peter Crosthwaite wrote: > On Sun, Aug 3, 2014 at 1:45 AM, Richard W.M. Jones <rjones@redhat.com> wrote: > > + max_bytes = UBOOT_MAX_GUNZIP_BYTES; > > Why does u-boot's maximum size limit apply here? We need some maximum to prevent people uploading a kernel (perhaps from an untrusted source) which is some sort of malicious gzip file that expands to a huge size. In this case the u-boot limit is 64 MB which is larger than most possible kernels, so it seemed like a reasonable limit to choose. You're right there is no connection to u-boot, except that both the -kernel option and u-boot have similar concerns with maximum kernel size, and presumably the u-boot limit is battle-tested. I'll split the patch into two and send v5 soon. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed kernel. 2014-08-04 8:48 ` Richard W.M. Jones @ 2014-08-04 8:52 ` Peter Maydell 2014-08-04 9:11 ` Peter Crosthwaite 1 sibling, 0 replies; 6+ messages in thread From: Peter Maydell @ 2014-08-04 8:52 UTC (permalink / raw) To: Richard W.M. Jones; +Cc: Peter Crosthwaite, qemu-devel@nongnu.org Developers On 4 August 2014 09:48, Richard W.M. Jones <rjones@redhat.com> wrote: > On Mon, Aug 04, 2014 at 09:05:39AM +1000, Peter Crosthwaite wrote: >> On Sun, Aug 3, 2014 at 1:45 AM, Richard W.M. Jones <rjones@redhat.com> wrote: >> > + max_bytes = UBOOT_MAX_GUNZIP_BYTES; >> >> Why does u-boot's maximum size limit apply here? > > We need some maximum to prevent people uploading a kernel (perhaps > from an untrusted source) which is some sort of malicious gzip file > that expands to a huge size. If we care about malicious zipfiles we should probably fix the bits in gunzip() which trust the gzip header more than they should... thanks -- PMM ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed kernel. 2014-08-04 8:48 ` Richard W.M. Jones 2014-08-04 8:52 ` Peter Maydell @ 2014-08-04 9:11 ` Peter Crosthwaite 1 sibling, 0 replies; 6+ messages in thread From: Peter Crosthwaite @ 2014-08-04 9:11 UTC (permalink / raw) To: Richard W.M. Jones; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers On Mon, Aug 4, 2014 at 6:48 PM, Richard W.M. Jones <rjones@redhat.com> wrote: > On Mon, Aug 04, 2014 at 09:05:39AM +1000, Peter Crosthwaite wrote: >> On Sun, Aug 3, 2014 at 1:45 AM, Richard W.M. Jones <rjones@redhat.com> wrote: >> > + max_bytes = UBOOT_MAX_GUNZIP_BYTES; >> >> Why does u-boot's maximum size limit apply here? > > We need some maximum to prevent people uploading a kernel (perhaps > from an untrusted source) which is some sort of malicious gzip file > that expands to a huge size. > > In this case the u-boot limit is 64 MB which is larger than most > possible kernels, so it seemed like a reasonable limit to choose. Ok. If you really do need this artificial limit then I think you should just make your own macro with the same value. Regards, Peter > You're right there is no connection to u-boot, except that both the > -kernel option and u-boot have similar concerns with maximum kernel > size, and presumably the u-boot limit is battle-tested. > > I'll split the patch into two and send v5 soon. > > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > libguestfs lets you edit virtual machines. Supports shell scripting, > bindings from many languages. http://libguestfs.org > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-08-04 9:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-08-02 15:45 [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed Richard W.M. Jones 2014-08-02 15:45 ` [Qemu-devel] [PATCH v4] aarch64: Allow -kernel option to take a gzip-compressed kernel Richard W.M. Jones 2014-08-03 23:05 ` Peter Crosthwaite 2014-08-04 8:48 ` Richard W.M. Jones 2014-08-04 8:52 ` Peter Maydell 2014-08-04 9:11 ` Peter Crosthwaite
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).