On Mon, 14 Feb 2022 10:47:43 +0100 Christian Schoenebeck <qemu_...@crudebyte.com> wrote:
> On Sonntag, 13. Februar 2022 21:33:10 CET Peter Maydell wrote: > > On Thu, 10 Feb 2022 at 11:33, Christian Schoenebeck > > > > <qemu_...@crudebyte.com> wrote: > > > The following changes since commit > > > 0a301624c2f4ced3331ffd5bce85b4274fe132af: > > > Merge remote-tracking branch > > > 'remotes/pmaydell/tags/pull-target-arm-20220208' into staging > > > (2022-02-08 11:40:08 +0000)> > > > are available in the Git repository at: > > > https://github.com/cschoenebeck/qemu.git tags/pull-9p-20220210 > > > > > > for you to fetch changes up to de19c79dad6a2cad54ae04ce754d47c07bf9bc93: > > > 9pfs: Fix segfault in do_readdir_many caused by struct dirent overread > > > (2022-02-10 11:56:01 +0100)> > > > ----------------------------------------------------------------9d82f6a3e68c2 > > > 9pfs: fixes and cleanup > > > > > > * Fifth patch fixes a 9pfs server crash that happened on some systems due > > > > > > to incorrect (system dependant) handling of struct dirent size. > > > > > > * Tests: Second patch fixes a test error that happened on some systems due > > > > > > mkdir() being called twice for creating the test directory for the 9p > > > 'local' tests. > > > > > > * Tests: Third patch fixes a memory leak. > > > > > > * Tests: The remaining two patches are code cleanup. > > > > > > ---------------------------------------------------------------- > > > > Hi; this fails CI for the build-oss-fuzz job, which finds > > a heap-buffer-overflow: > > https://gitlab.com/qemu-project/qemu/-/jobs/2087610013 > > So this is about the 'dirent' patch: > https://github.com/cschoenebeck/qemu/commit/de19c79dad6a2cad54ae04ce754d47c07bf9bc93 > > In conjunction with the 9p fuzzing tests: > https://wiki.qemu.org/Documentation/9p#Fuzzing > > I first thought it might be a false positive due to the unorthodox handling of > dirent duplication by that patch, but from the ASan output below I am not > really sure about that. > No, this is an actual bug. See below. > Is there a way to get the content of local variables? > > Would it be possible that the following issue (g_memdup vs. g_memdup2) might > apply here? > https://discourse.gnome.org/t/port-your-module-from-g-memdup-to-g-memdup2-now/5538 > > Best regards, > Christian Schoenebeck > > > > > 8/152 qemu:qtest+qtest-i386 / qtest-i386/qos-test ERROR 66.74s killed > > by signal 6 SIGABRT > > > > >>> QTEST_QEMU_BINARY=./qemu-system-i386 > > >>> QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon > > >>> MALLOC_PERTURB_=120 > > >>> G_TEST_DBUS_DAEMON=/builds/qemu-project/qemu/tests/dbus-vmstate-daemon. > > >>> sh QTEST_QEMU_IMG=./qemu-img > > >>> /builds/qemu-project/qemu/build-oss-fuzz/tests/qtest/qos-test --tap -k > > ――――――――――――――――――――――――――――――――――――― ✀ > > ――――――――――――――――――――――――――――――――――――― Listing only the last 100 lines from > > a long log. > > For details see https://github.com/google/sanitizers/issues/189 > > ==7270==WARNING: ASan doesn't fully support makecontext/swapcontext > > functions and may produce false positives in some cases! > > ==7270==WARNING: ASan is ignoring requested __asan_handle_no_return: > > stack type: default top: 0x7ffc79fb0000; bottom 0x7ff908ffd000; size: > > 0x000370fb3000 (14780411904) > > False positive error reports may follow > > For details see https://github.com/google/sanitizers/issues/189 > > ==7276==WARNING: ASan doesn't fully support makecontext/swapcontext > > functions and may produce false positives in some cases! > > ==7276==WARNING: ASan is ignoring requested __asan_handle_no_return: > > stack type: default top: 0x7fff7e4a8000; bottom 0x7fd6363fd000; size: > > 0x0029480ab000 (177302319104) > > False positive error reports may follow > > For details see https://github.com/google/sanitizers/issues/189 > > ==7282==WARNING: ASan doesn't fully support makecontext/swapcontext > > functions and may produce false positives in some cases! > > ==7282==WARNING: ASan is ignoring requested __asan_handle_no_return: > > stack type: default top: 0x7ffee6e7f000; bottom 0x7f32fb5fd000; size: > > 0x00cbeb882000 (875829927936) > > False positive error reports may follow > > For details see https://github.com/google/sanitizers/issues/189 > > ==7288==WARNING: ASan doesn't fully support makecontext/swapcontext > > functions and may produce false positives in some cases! > > ==7288==WARNING: ASan is ignoring requested __asan_handle_no_return: > > stack type: default top: 0x7ffc6118e000; bottom 0x7f6391cfd000; size: > > 0x0098cf491000 (656312700928) > > False positive error reports may follow > > For details see https://github.com/google/sanitizers/issues/189 > > ==7294==WARNING: ASan doesn't fully support makecontext/swapcontext > > functions and may produce false positives in some cases! > > ==7294==WARNING: ASan is ignoring requested __asan_handle_no_return: > > stack type: default top: 0x7ffef665d000; bottom 0x7f69dc8fd000; size: > > 0x009519d60000 (640383582208) > > False positive error reports may follow > > For details see https://github.com/google/sanitizers/issues/189 > > ==7300==WARNING: ASan doesn't fully support makecontext/swapcontext > > functions and may produce false positives in some cases! > > ==7300==WARNING: ASan is ignoring requested __asan_handle_no_return: > > stack type: default top: 0x7ffe33db0000; bottom 0x7f01421fd000; size: > > 0x00fcf1bb3000 (1086387335168) > > False positive error reports may follow > > For details see https://github.com/google/sanitizers/issues/189 > > ==7306==WARNING: ASan doesn't fully support makecontext/swapcontext > > functions and may produce false positives in some cases! > > ==7306==WARNING: ASan is ignoring requested __asan_handle_no_return: > > stack type: default top: 0x7ffebd618000; bottom 0x7ff1179fd000; size: > > 0x000da5c1b000 (58615508992) > > False positive error reports may follow > > For details see https://github.com/google/sanitizers/issues/189 > > ================================================================= > > ==7306==ERROR: AddressSanitizer: heap-buffer-overflow on address > > 0x612000030768 at pc 0x562351066c74 bp 0x7ff1078c3a90 sp > > 0x7ff1078c3240 > > READ of size 48830 at 0x612000030768 thread T4 The size looks pretty big to me. Test file paths in virtio-9p-test are only a couple of bytes long... > > #0 0x562351066c73 in __interceptor_memcpy.part.0 asan_interceptors.cpp.o > > #1 0x7ff1290d443f in g_memdup (/lib64/libglib-2.0.so.0+0x6e43f) > > #2 0x56235134537a in do_readdir_many > > /builds/qemu-project/qemu/build-oss-fuzz/../hw/9pfs/codir.c:146:19 > > #3 0x56235134537a in v9fs_co_readdir_many > > /builds/qemu-project/qemu/build-oss-fuzz/../hw/9pfs/codir.c:225:5 > > #4 0x56235132d626 in v9fs_do_readdir > > /builds/qemu-project/qemu/build-oss-fuzz/../hw/9pfs/9p.c:2430:13 > > #5 0x56235132d626 in v9fs_readdir > > /builds/qemu-project/qemu/build-oss-fuzz/../hw/9pfs/9p.c:2543:13 > > #6 0x56235257101e in coroutine_trampoline > > /builds/qemu-project/qemu/build-oss-fuzz/../util/coroutine-ucontext.c:173:9 > > #7 0x7ff126e0e84f (/lib64/libc.so.6+0x5784f) > > 0x612000030768 is located 0 bytes to the right of 296-byte region > > [0x612000030640,0x612000030768) > > allocated by thread T4 here: > > #0 0x5623510a4e47 in malloc > > (/builds/qemu-project/qemu/build-oss-fuzz/qemu-system-i386+0x1146e47) > > #1 0x7ff1290c03d8 in g_malloc (/lib64/libglib-2.0.so.0+0x5a3d8) i.e. synth_open = g_malloc(sizeof(*synth_open)); and we have: typedef struct V9fsSynthOpenState { off_t offset; V9fsSynthNode *node; struct dirent dent; } V9fsSynthOpenState; It looks like that qemu_dirent_dup() ends up using an uninitialized dent->d_reclen. The synth backend should be fixed to honor d_reclen, or at least to allocate with g_new0(). Cheers, -- Greg > > #2 0x56235131e659 in synth_opendir > > /builds/qemu-project/qemu/build-oss-fuzz/../hw/9pfs/9p-synth.c:185:18 > > #3 0x5623513462f5 in v9fs_co_opendir > > /builds/qemu-project/qemu/build-oss-fuzz/../hw/9pfs/codir.c:321:5 > > #4 0x5623513257d7 in v9fs_open > > /builds/qemu-project/qemu/build-oss-fuzz/../hw/9pfs/9p.c:1959:15 > > #5 0x56235257101e in coroutine_trampoline > > /builds/qemu-project/qemu/build-oss-fuzz/../util/coroutine-ucontext.c:173:9 > > #6 0x7ff126e0e84f (/lib64/libc.so.6+0x5784f) > > Thread T4 created by T0 here: > > #0 0x562351015926 in pthread_create > > (/builds/qemu-project/qemu/build-oss-fuzz/qemu-system-i386+0x10b7926) > > #1 0x5623525351ea in qemu_thread_create > > /builds/qemu-project/qemu/build-oss-fuzz/../util/qemu-thread-posix.c:596:11 > > #2 0x5623525a4588 in do_spawn_thread > > /builds/qemu-project/qemu/build-oss-fuzz/../util/thread-pool.c:134:5 > > #3 0x5623525a4588 in spawn_thread_bh_fn > > /builds/qemu-project/qemu/build-oss-fuzz/../util/thread-pool.c:142:5 > > #4 0x562352569814 in aio_bh_call > > /builds/qemu-project/qemu/build-oss-fuzz/../util/async.c:141:5 > > #5 0x562352569814 in aio_bh_poll > > /builds/qemu-project/qemu/build-oss-fuzz/../util/async.c:169:13 > > #6 0x5623525248cc in aio_dispatch > > /builds/qemu-project/qemu/build-oss-fuzz/../util/aio-posix.c:415:5 > > #7 0x56235256c34c in aio_ctx_dispatch > > /builds/qemu-project/qemu/build-oss-fuzz/../util/async.c:311:5 > > #8 0x7ff1290bb05e in g_main_context_dispatch > > (/lib64/libglib-2.0.so.0+0x5505e) SUMMARY: AddressSanitizer: > > heap-buffer-overflow > > asan_interceptors.cpp.o in __interceptor_memcpy.part.0 > > Shadow bytes around the buggy address: > > 0x0c247fffe090: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd > > 0x0c247fffe0a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd > > 0x0c247fffe0b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa > > 0x0c247fffe0c0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00 > > 0x0c247fffe0d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > =>0x0c247fffe0e0: 00 00 00 00 00 00 00 00 00 00 00 00 00[fa]fa fa > > 0x0c247fffe0f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa > > 0x0c247fffe100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa > > 0x0c247fffe110: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa > > 0x0c247fffe120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa > > 0x0c247fffe130: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa > > Shadow byte legend (one shadow byte represents 8 application bytes): > > Addressable: 00 > > Partially addressable: 01 02 03 04 05 06 07 > > Heap left redzone: fa > > Freed heap region: fd > > Stack left redzone: f1 > > Stack mid redzone: f2 > > Stack right redzone: f3 > > Stack after return: f5 > > Stack use after scope: f8 > > Global redzone: f9 > > Global init order: f6 > > Poisoned by user: f7 > > Container overflow: fc > > Array cookie: ac > > Intra object redzone: bb > > ASan internal: fe > > Left alloca redzone: ca > > Right alloca redzone: cb > > ==7306==ABORTING > > > > > > thanks > > -- PMM > >