On 8/12/21 4:18 PM, Peter Maydell wrote: > The gunzip() function reads various fields from a passed in source > buffer in order to skip a header before passing the actual compressed > data to the zlib inflate() function. It does check whether the > passed in buffer is too small, but unfortunately it checks that only > after reading bytes from the src buffer, so it could read off the end > of the buffer. > > You can see this with valgrind: > > $ printf "%b" '\x1f\x8b' > /tmp/image > $ valgrind qemu-system-aarch64 -display none -M virt -cpu max -kernel > /tmp/image
Nice :) Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> > [...] > ==19224== Invalid read of size 1 > ==19224== at 0x67302E: gunzip (loader.c:558) > ==19224== by 0x673907: load_image_gzipped_buffer (loader.c:788) > ==19224== by 0xA18032: load_aarch64_image (boot.c:932) > ==19224== by 0xA18489: arm_setup_direct_kernel_boot (boot.c:1063) > ==19224== by 0xA18D90: arm_load_kernel (boot.c:1317) > ==19224== by 0x9F3651: machvirt_init (virt.c:2114) > ==19224== by 0x794B7A: machine_run_board_init (machine.c:1272) > ==19224== by 0xD5CAD3: qemu_init_board (vl.c:2618) > ==19224== by 0xD5CCA6: qmp_x_exit_preconfig (vl.c:2692) > ==19224== by 0xD5F32E: qemu_init (vl.c:3713) > ==19224== by 0x5ADDB1: main (main.c:49) > ==19224== Address 0x3802a873 is 0 bytes after a block of size 3 alloc'd > ==19224== at 0x4C31B0F: malloc (in > /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) > ==19224== by 0x61E7657: g_file_get_contents (in > /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.5600.4) > ==19224== by 0x673895: load_image_gzipped_buffer (loader.c:771) > ==19224== by 0xA18032: load_aarch64_image (boot.c:932) > ==19224== by 0xA18489: arm_setup_direct_kernel_boot (boot.c:1063) > ==19224== by 0xA18D90: arm_load_kernel (boot.c:1317) > ==19224== by 0x9F3651: machvirt_init (virt.c:2114) > ==19224== by 0x794B7A: machine_run_board_init (machine.c:1272) > ==19224== by 0xD5CAD3: qemu_init_board (vl.c:2618) > ==19224== by 0xD5CCA6: qmp_x_exit_preconfig (vl.c:2692) > ==19224== by 0xD5F32E: qemu_init (vl.c:3713) > ==19224== by 0x5ADDB1: main (main.c:49) > > Check that we have enough bytes of data to read the header bytes that > we read before we read them. > > Fixes: Coverity 1458997 > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > --- > hw/core/loader.c | 35 +++++++++++++++++++++++++---------- > 1 file changed, 25 insertions(+), 10 deletions(-)
