From: "Daniel P. Berrangé" <berrange@redhat.com> To: Peter Maydell <peter.maydell@linaro.org> Cc: Juan Quintela <quintela@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Thomas Huth <thuth@redhat.com>, kvm-devel <kvm@vger.kernel.org>, QEMU Developers <qemu-devel@nongnu.org>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net> Subject: Re: [Qemu-devel] [PULL 00/19] Migration patches Date: Mon, 15 Jul 2019 15:04:41 +0100 [thread overview] Message-ID: <20190715140441.GJ30298@redhat.com> (raw) In-Reply-To: <CAFEAcA9ncjtGdc8CZOJBDBRtzEU8oL7YicVg5PtyiiO2O4z51w@mail.gmail.com> On Mon, Jul 15, 2019 at 12:16:57PM +0100, Peter Maydell wrote: > On Fri, 12 Jul 2019 at 17:33, Peter Maydell <peter.maydell@linaro.org> wrote: > > Still fails on aarch32 host, I'm afraid: > > > > MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} > > QTEST_QEMU_BINARY=aarch64-softmmu/qemu-system-aarch64 > > QTEST_QEMU_IMG=qemu-img tests/migration-test -m=quick -k --tap < > > /dev/null | ./scripts/tap-driver.pl --test-name="migration-test" > > PASS 1 migration-test /aarch64/migration/deprecated > > PASS 2 migration-test /aarch64/migration/bad_dest > > PASS 3 migration-test /aarch64/migration/fd_proto > > PASS 4 migration-test /aarch64/migration/postcopy/unix > > PASS 5 migration-test /aarch64/migration/postcopy/recovery > > PASS 6 migration-test /aarch64/migration/precopy/unix > > PASS 7 migration-test /aarch64/migration/precopy/tcp > > PASS 8 migration-test /aarch64/migration/xbzrle/unix > > malloc(): memory corruption > > Broken pipe > > qemu-system-aarch64: load of migration failed: Invalid argument > > /home/peter.maydell/qemu/tests/libqtest.c:137: kill_qemu() tried to > > terminate QEMU process but encountered exit status 1 > > Aborted > > ERROR - too few tests run (expected 9, got 8) > > /home/peter.maydell/qemu/tests/Makefile.include:899: recipe for target > > 'check-qtest-aarch64' failed > > A run with valgrind: > > (armhf)pmaydell@mustang-maydell:~/qemu/build/all-a32$ > QTEST_QEMU_BINARY='valgrind aarch64-softmmu/qemu-system-aarch64' > tests/migration-test -v -p '/aarch64/migration/multifd/tcp' > /aarch64/migration/multifd/tcp: ==4034== Memcheck, a memory error detector > ==4034== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. > ==4034== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info > ==4034== Command: aarch64-softmmu/qemu-system-aarch64 -qtest > unix:/tmp/qtest-4033.sock -qtest-log /dev/null -chardev > socket,path=/tmp/qtest-4033.qmp,id=char0 -mon > chardev=char0,mode=control -machine accel=qtest -display none -machine > virt,accel=kvm:tcg,gic-version=max -name vmsource,debug-threads=on > -cpu max -m 150M -serial file:/tmp/migration-test-mSLr4A/src_serial > -kernel /tmp/migration-test-mSLr4A/bootsect > ==4034== > ==4040== Memcheck, a memory error detector > ==4040== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. > ==4040== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info > ==4040== Command: aarch64-softmmu/qemu-system-aarch64 -qtest > unix:/tmp/qtest-4033.sock -qtest-log /dev/null -chardev > socket,path=/tmp/qtest-4033.qmp,id=char0 -mon > chardev=char0,mode=control -machine accel=qtest -display none -machine > virt,accel=kvm:tcg,gic-version=max -name vmdest,debug-threads=on -cpu > max -m 150M -serial file:/tmp/migration-test-mSLr4A/dest_serial > -kernel /tmp/migration-test-mSLr4A/bootsect -incoming tcp:127.0.0.1:0 > ==4040== > ==4034== Thread 5 multifdsend_0: > ==4034== Syscall param sendmsg(msg.msg_iov[0]) points to uninitialised byte(s) > ==4034== at 0x5299F06: __libc_do_syscall (libc-do-syscall.S:47) > ==4034== by 0x5298FCB: sendmsg (sendmsg.c:28) > ==4034== by 0x60135D: qio_channel_socket_writev (channel-socket.c:544) > ==4034== by 0x5FF995: qio_channel_writev (channel.c:207) > ==4034== by 0x5FF995: qio_channel_writev_all (channel.c:171) > ==4034== by 0x5FFA0F: qio_channel_write_all (channel.c:257) > ==4034== by 0x26BA73: multifd_send_initial_packet (ram.c:711) > ==4034== by 0x26BA73: multifd_send_thread (ram.c:1085) > ==4034== by 0x63C0B1: qemu_thread_start (qemu-thread-posix.c:502) > ==4034== by 0x5290613: start_thread (pthread_create.c:463) > ==4034== by 0x53487FB: ??? (clone.S:73) > ==4034== Address 0x2320048d is on thread 5's stack > ==4034== in frame #5, created by multifd_send_thread (ram.c:1077) This is a simple missing initialization multifd_send_initial_packet has a local variable: MultiFDInit_t msg; the code initializes 4 fields, but does *not* initialize the 2 padding fields, so we're writing random data. Harmless as the receiving end will ignore padding too, but we should fill with zeros really. so MultiFDInit_t msg = {0}; should fix it. > ==4034== > ==4034== Thread 6 multifdsend_1: > ==4034== Invalid write of size 4 > ==4034== at 0x26BB7C: multifd_send_fill_packet (ram.c:806) > ==4034== by 0x26BB7C: multifd_send_thread (ram.c:1101) > ==4034== by 0x63C0B1: qemu_thread_start (qemu-thread-posix.c:502) > ==4034== by 0x5290613: start_thread (pthread_create.c:463) > ==4034== by 0x53487FB: ??? (clone.S:73) > ==4034== Address 0x224ed668 is 0 bytes after a block of size 832 alloc'd > ==4034== at 0x4841BC4: calloc (vg_replace_malloc.c:711) > ==4034== by 0x5018269: g_malloc0 (in > /usr/lib/arm-linux-gnueabihf/libglib-2.0.so.0.5600.4) multifd_send_fill_packet is getting the oob write in: for (i = 0; i < p->pages->used; i++) { packet->offset[i] = cpu_to_be64(p->pages->offset[i]); } offset is a variable length struct field at the end of MultiFDPacket_t: typedef struct { ...snip... char ramblock[256]; uint64_t offset[]; } __attribute__((packed)) MultiFDPacket_t; but the packet data is allocated back in multifd_save_setup using: p->packet_len = sizeof(MultiFDPacket_t) + sizeof(ram_addr_t) * page_count; p->packet = g_malloc0(p->packet_len); Notice the field in the struct is "uint64_t" but the length we're allocating is "ram_addr_t". Since this is a 32-bit build, I'm guessing ram_addr_t is a 32-bit integer and thus we're under-allocating the variable length offset field by half Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
WARNING: multiple messages have this Message-ID (diff)
From: "Daniel P. Berrangé" <berrange@redhat.com> To: Peter Maydell <peter.maydell@linaro.org> Cc: Laurent Vivier <lvivier@redhat.com>, Thomas Huth <thuth@redhat.com>, kvm-devel <kvm@vger.kernel.org>, Juan Quintela <quintela@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, QEMU Developers <qemu-devel@nongnu.org>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net> Subject: Re: [Qemu-devel] [PULL 00/19] Migration patches Date: Mon, 15 Jul 2019 15:04:41 +0100 [thread overview] Message-ID: <20190715140441.GJ30298@redhat.com> (raw) In-Reply-To: <CAFEAcA9ncjtGdc8CZOJBDBRtzEU8oL7YicVg5PtyiiO2O4z51w@mail.gmail.com> On Mon, Jul 15, 2019 at 12:16:57PM +0100, Peter Maydell wrote: > On Fri, 12 Jul 2019 at 17:33, Peter Maydell <peter.maydell@linaro.org> wrote: > > Still fails on aarch32 host, I'm afraid: > > > > MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))} > > QTEST_QEMU_BINARY=aarch64-softmmu/qemu-system-aarch64 > > QTEST_QEMU_IMG=qemu-img tests/migration-test -m=quick -k --tap < > > /dev/null | ./scripts/tap-driver.pl --test-name="migration-test" > > PASS 1 migration-test /aarch64/migration/deprecated > > PASS 2 migration-test /aarch64/migration/bad_dest > > PASS 3 migration-test /aarch64/migration/fd_proto > > PASS 4 migration-test /aarch64/migration/postcopy/unix > > PASS 5 migration-test /aarch64/migration/postcopy/recovery > > PASS 6 migration-test /aarch64/migration/precopy/unix > > PASS 7 migration-test /aarch64/migration/precopy/tcp > > PASS 8 migration-test /aarch64/migration/xbzrle/unix > > malloc(): memory corruption > > Broken pipe > > qemu-system-aarch64: load of migration failed: Invalid argument > > /home/peter.maydell/qemu/tests/libqtest.c:137: kill_qemu() tried to > > terminate QEMU process but encountered exit status 1 > > Aborted > > ERROR - too few tests run (expected 9, got 8) > > /home/peter.maydell/qemu/tests/Makefile.include:899: recipe for target > > 'check-qtest-aarch64' failed > > A run with valgrind: > > (armhf)pmaydell@mustang-maydell:~/qemu/build/all-a32$ > QTEST_QEMU_BINARY='valgrind aarch64-softmmu/qemu-system-aarch64' > tests/migration-test -v -p '/aarch64/migration/multifd/tcp' > /aarch64/migration/multifd/tcp: ==4034== Memcheck, a memory error detector > ==4034== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. > ==4034== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info > ==4034== Command: aarch64-softmmu/qemu-system-aarch64 -qtest > unix:/tmp/qtest-4033.sock -qtest-log /dev/null -chardev > socket,path=/tmp/qtest-4033.qmp,id=char0 -mon > chardev=char0,mode=control -machine accel=qtest -display none -machine > virt,accel=kvm:tcg,gic-version=max -name vmsource,debug-threads=on > -cpu max -m 150M -serial file:/tmp/migration-test-mSLr4A/src_serial > -kernel /tmp/migration-test-mSLr4A/bootsect > ==4034== > ==4040== Memcheck, a memory error detector > ==4040== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. > ==4040== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info > ==4040== Command: aarch64-softmmu/qemu-system-aarch64 -qtest > unix:/tmp/qtest-4033.sock -qtest-log /dev/null -chardev > socket,path=/tmp/qtest-4033.qmp,id=char0 -mon > chardev=char0,mode=control -machine accel=qtest -display none -machine > virt,accel=kvm:tcg,gic-version=max -name vmdest,debug-threads=on -cpu > max -m 150M -serial file:/tmp/migration-test-mSLr4A/dest_serial > -kernel /tmp/migration-test-mSLr4A/bootsect -incoming tcp:127.0.0.1:0 > ==4040== > ==4034== Thread 5 multifdsend_0: > ==4034== Syscall param sendmsg(msg.msg_iov[0]) points to uninitialised byte(s) > ==4034== at 0x5299F06: __libc_do_syscall (libc-do-syscall.S:47) > ==4034== by 0x5298FCB: sendmsg (sendmsg.c:28) > ==4034== by 0x60135D: qio_channel_socket_writev (channel-socket.c:544) > ==4034== by 0x5FF995: qio_channel_writev (channel.c:207) > ==4034== by 0x5FF995: qio_channel_writev_all (channel.c:171) > ==4034== by 0x5FFA0F: qio_channel_write_all (channel.c:257) > ==4034== by 0x26BA73: multifd_send_initial_packet (ram.c:711) > ==4034== by 0x26BA73: multifd_send_thread (ram.c:1085) > ==4034== by 0x63C0B1: qemu_thread_start (qemu-thread-posix.c:502) > ==4034== by 0x5290613: start_thread (pthread_create.c:463) > ==4034== by 0x53487FB: ??? (clone.S:73) > ==4034== Address 0x2320048d is on thread 5's stack > ==4034== in frame #5, created by multifd_send_thread (ram.c:1077) This is a simple missing initialization multifd_send_initial_packet has a local variable: MultiFDInit_t msg; the code initializes 4 fields, but does *not* initialize the 2 padding fields, so we're writing random data. Harmless as the receiving end will ignore padding too, but we should fill with zeros really. so MultiFDInit_t msg = {0}; should fix it. > ==4034== > ==4034== Thread 6 multifdsend_1: > ==4034== Invalid write of size 4 > ==4034== at 0x26BB7C: multifd_send_fill_packet (ram.c:806) > ==4034== by 0x26BB7C: multifd_send_thread (ram.c:1101) > ==4034== by 0x63C0B1: qemu_thread_start (qemu-thread-posix.c:502) > ==4034== by 0x5290613: start_thread (pthread_create.c:463) > ==4034== by 0x53487FB: ??? (clone.S:73) > ==4034== Address 0x224ed668 is 0 bytes after a block of size 832 alloc'd > ==4034== at 0x4841BC4: calloc (vg_replace_malloc.c:711) > ==4034== by 0x5018269: g_malloc0 (in > /usr/lib/arm-linux-gnueabihf/libglib-2.0.so.0.5600.4) multifd_send_fill_packet is getting the oob write in: for (i = 0; i < p->pages->used; i++) { packet->offset[i] = cpu_to_be64(p->pages->offset[i]); } offset is a variable length struct field at the end of MultiFDPacket_t: typedef struct { ...snip... char ramblock[256]; uint64_t offset[]; } __attribute__((packed)) MultiFDPacket_t; but the packet data is allocated back in multifd_save_setup using: p->packet_len = sizeof(MultiFDPacket_t) + sizeof(ram_addr_t) * page_count; p->packet = g_malloc0(p->packet_len); Notice the field in the struct is "uint64_t" but the length we're allocating is "ram_addr_t". Since this is a 32-bit build, I'm guessing ram_addr_t is a 32-bit integer and thus we're under-allocating the variable length offset field by half Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
next prev parent reply other threads:[~2019-07-15 14:06 UTC|newest] Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-12 14:31 [PULL 00/19] Migration patches Juan Quintela 2019-07-12 14:31 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:31 ` [PULL 01/19] migration: fix multifd_recv event typo Juan Quintela 2019-07-12 14:31 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:31 ` [PULL 02/19] migration-test: rename parameter to parameter_int Juan Quintela 2019-07-12 14:31 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:31 ` [PULL 03/19] migration-test: Add migration multifd test Juan Quintela 2019-07-12 14:31 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:31 ` [PULL 04/19] migration/multifd: call multifd_send_sync_main when sending RAM_SAVE_FLAG_EOS Juan Quintela 2019-07-12 14:31 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:31 ` [PULL 05/19] migration/xbzrle: update cache and current_data in one place Juan Quintela 2019-07-12 14:31 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:31 ` [PULL 06/19] cutils: remove one unnecessary pointer operation Juan Quintela 2019-07-12 14:31 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:31 ` [PULL 07/19] migration/multifd: sync packet_num after all thread are done Juan Quintela 2019-07-12 14:31 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:31 ` [PULL 08/19] migration/ram.c: reset complete_round when we gets a queued page Juan Quintela 2019-07-12 14:31 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:31 ` [PULL 09/19] migration: No need to take rcu during sync_dirty_bitmap Juan Quintela 2019-07-12 14:31 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:31 ` [PULL 10/19] memory: Don't set migration bitmap when without migration Juan Quintela 2019-07-12 14:31 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:31 ` [PULL 11/19] bitmap: Add bitmap_copy_with_{src|dst}_offset() Juan Quintela 2019-07-12 14:31 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:32 ` [PULL 12/19] memory: Pass mr into snapshot_and_clear_dirty Juan Quintela 2019-07-12 14:32 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:32 ` [PULL 13/19] memory: Introduce memory listener hook log_clear() Juan Quintela 2019-07-12 14:32 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:32 ` [PULL 14/19] kvm: Update comments for sync_dirty_bitmap Juan Quintela 2019-07-12 14:32 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:32 ` [PULL 15/19] kvm: Persistent per kvmslot dirty bitmap Juan Quintela 2019-07-12 14:32 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:32 ` [PULL 16/19] kvm: Introduce slots lock for memory listener Juan Quintela 2019-07-12 14:32 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:32 ` [PULL 17/19] kvm: Support KVM_CLEAR_DIRTY_LOG Juan Quintela 2019-07-12 14:32 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:32 ` [PULL 18/19] migration: Split log_clear() into smaller chunks Juan Quintela 2019-07-12 14:32 ` [Qemu-devel] " Juan Quintela 2019-07-12 14:32 ` [PULL 19/19] migration: allow private destination ram with x-ignore-shared Juan Quintela 2019-07-12 14:32 ` [Qemu-devel] " Juan Quintela 2019-07-12 16:33 ` [Qemu-devel] [PULL 00/19] Migration patches Peter Maydell 2019-07-12 16:33 ` Peter Maydell 2019-07-12 17:54 ` Dr. David Alan Gilbert 2019-07-12 17:54 ` Dr. David Alan Gilbert 2019-07-15 11:16 ` Peter Maydell 2019-07-15 11:16 ` Peter Maydell 2019-07-15 13:44 ` Juan Quintela 2019-07-15 13:44 ` [Qemu-devel] " Juan Quintela 2019-07-15 13:48 ` Peter Maydell 2019-07-15 13:48 ` [Qemu-devel] " Peter Maydell 2019-07-15 14:10 ` Juan Quintela 2019-07-15 14:10 ` [Qemu-devel] " Juan Quintela 2019-07-15 14:15 ` Peter Maydell 2019-07-15 14:15 ` [Qemu-devel] " Peter Maydell 2019-07-15 14:04 ` Daniel P. Berrangé [this message] 2019-07-15 14:04 ` Daniel P. Berrangé 2019-07-15 14:17 ` Peter Maydell 2019-07-15 14:17 ` Peter Maydell -- strict thread matches above, loose matches on Subject: below -- 2019-07-11 10:43 Juan Quintela 2019-07-11 11:19 ` Paolo Bonzini 2019-07-11 11:32 ` Juan Quintela 2019-07-11 11:34 ` Dr. David Alan Gilbert 2019-07-11 11:40 ` Peter Maydell 2019-07-11 11:40 ` Peter Maydell 2019-07-11 12:39 ` Peter Maydell 2019-07-11 12:39 ` Peter Maydell 2019-07-12 14:06 ` Juan Quintela 2019-07-11 12:55 ` Christian Borntraeger 2019-07-11 13:01 ` Peter Maydell 2019-07-11 13:01 ` Peter Maydell 2019-07-11 13:00 ` no-reply 2019-07-11 13:00 ` no-reply 2019-07-12 14:33 ` no-reply 2019-07-12 14:33 ` no-reply
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190715140441.GJ30298@redhat.com \ --to=berrange@redhat.com \ --cc=dgilbert@redhat.com \ --cc=kvm@vger.kernel.org \ --cc=lvivier@redhat.com \ --cc=pbonzini@redhat.com \ --cc=peter.maydell@linaro.org \ --cc=qemu-devel@nongnu.org \ --cc=quintela@redhat.com \ --cc=rth@twiddle.net \ --cc=thuth@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.