Skip to content

Commit

Permalink
tests/migration-test: Fix read off end of aarch64_kernel array
Browse files Browse the repository at this point in the history
The test aarch64 kernel is in an array defined with
 unsigned char aarch64_kernel[] = { [...] }

which means it could be any size; currently it's quite small.
However we write it to a file using init_bootfile(), which
writes exactly 512 bytes to the file. This will break if
we ever end up with a kernel larger than that, and will
read garbage off the end of the array in the current setup
where the kernel is smaller.

Make init_bootfile() take an argument giving the length of
the data to write. This allows us to use it for all architectures
(previously s390 had a special-purpose init_bootfile_s390x
which hardcoded the file to write so it could write the
correct length). We assert that the x86 bootfile really is
exactly 512 bytes as it should be (and as we were previously
just assuming it was).

This was detected by the clang-7 asan:
==15607==ERROR: AddressSanitizer: global-buffer-overflow on address 0x55a796f51d20 at pc 0x55a796b89c2f bp 0x7ffc58e89160 sp 0x7ffc58e88908
READ of size 512 at 0x55a796f51d20 thread T0
    #0 0x55a796b89c2e in fwrite (/home/petmay01/linaro/qemu-from-laptop/qemu/build/sanitizers/tests/migration-test+0xb0c2e)
    #1 0x55a796c46492 in init_bootfile /home/petmay01/linaro/qemu-from-laptop/qemu/tests/migration-test.c:99:5
    #2 0x55a796c46492 in test_migrate_start /home/petmay01/linaro/qemu-from-laptop/qemu/tests/migration-test.c:593
    qemu#3 0x55a796c44101 in test_baddest /home/petmay01/linaro/qemu-from-laptop/qemu/tests/migration-test.c:854:9
    qemu#4 0x7f906ffd3cc9  (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x72cc9)
    qemu#5 0x7f906ffd3bfa  (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x72bfa)
    qemu#6 0x7f906ffd3bfa  (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x72bfa)
    qemu#7 0x7f906ffd3ea1 in g_test_run_suite (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x72ea1)
    qemu#8 0x7f906ffd3ec0 in g_test_run (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x72ec0)
    qemu#9 0x55a796c43707 in main /home/petmay01/linaro/qemu-from-laptop/qemu/tests/migration-test.c:1187:11
    qemu#10 0x7f906e9abb96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    qemu#11 0x55a796b6c2d9 in _start (/home/petmay01/linaro/qemu-from-laptop/qemu/build/sanitizers/tests/migration-test+0x932d9)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20190702150311.20467-1-peter.maydell@linaro.org
  • Loading branch information
pm215 committed Jul 8, 2019
1 parent 6a02a73 commit 2785f19
Showing 1 changed file with 7 additions and 15 deletions.
22 changes: 7 additions & 15 deletions tests/migration-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -91,23 +91,13 @@ static const char *tmpfs;
*/
#include "tests/migration/i386/a-b-bootblock.h"
#include "tests/migration/aarch64/a-b-kernel.h"

static void init_bootfile(const char *bootpath, void *content)
{
FILE *bootfile = fopen(bootpath, "wb");

g_assert_cmpint(fwrite(content, 512, 1, bootfile), ==, 1);
fclose(bootfile);
}

#include "tests/migration/s390x/a-b-bios.h"

static void init_bootfile_s390x(const char *bootpath)
static void init_bootfile(const char *bootpath, void *content, size_t len)
{
FILE *bootfile = fopen(bootpath, "wb");
size_t len = sizeof(s390x_elf);

g_assert_cmpint(fwrite(s390x_elf, len, 1, bootfile), ==, 1);
g_assert_cmpint(fwrite(content, len, 1, bootfile), ==, 1);
fclose(bootfile);
}

Expand Down Expand Up @@ -537,7 +527,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
got_stop = false;
bootpath = g_strdup_printf("%s/bootsect", tmpfs);
if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
init_bootfile(bootpath, x86_bootsect);
/* the assembled x86 boot sector should be exactly one sector large */
assert(sizeof(x86_bootsect) == 512);
init_bootfile(bootpath, x86_bootsect, sizeof(x86_bootsect));
extra_opts = use_shmem ? get_shmem_opts("150M", shmem_path) : NULL;
cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
" -name source,debug-threads=on"
Expand All @@ -555,7 +547,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
start_address = X86_TEST_MEM_START;
end_address = X86_TEST_MEM_END;
} else if (g_str_equal(arch, "s390x")) {
init_bootfile_s390x(bootpath);
init_bootfile(bootpath, s390x_elf, sizeof(s390x_elf));
extra_opts = use_shmem ? get_shmem_opts("128M", shmem_path) : NULL;
cmd_src = g_strdup_printf("-machine accel=%s -m 128M"
" -name source,debug-threads=on"
Expand Down Expand Up @@ -590,7 +582,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
start_address = PPC_TEST_MEM_START;
end_address = PPC_TEST_MEM_END;
} else if (strcmp(arch, "aarch64") == 0) {
init_bootfile(bootpath, aarch64_kernel);
init_bootfile(bootpath, aarch64_kernel, sizeof(aarch64_kernel));
extra_opts = use_shmem ? get_shmem_opts("150M", shmem_path) : NULL;
cmd_src = g_strdup_printf("-machine virt,accel=%s,gic-version=max "
"-name vmsource,debug-threads=on -cpu max "
Expand Down

0 comments on commit 2785f19

Please sign in to comment.