Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

synopsys designware ethernet gmac #5

Open
ho28 opened this issue Sep 14, 2024 · 0 comments
Open

synopsys designware ethernet gmac #5

ho28 opened this issue Sep 14, 2024 · 0 comments

Comments

@ho28
Copy link
Collaborator

ho28 commented Sep 14, 2024

The s32g machine currently uses mmio virtio-net for networking. This issue tracks development of emulated synopsys DW GMAC device implementation and integration into s32g machine
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Nov 15, 2024
@foxmox
block/reqlist: allow adding overlapping requests
7eefbf8 
Allow overlapping request by removing the assert that made it
impossible. There are only two callers:

1. block_copy_task_create()

It already asserts the very same condition before calling
reqlist_init_req().

2. cbw_snapshot_read_lock()

There is no need to have read requests be non-overlapping in
copy-before-write when used for snapshot-access. In fact, there was no
protection against two callers of cbw_snapshot_read_lock() calling
reqlist_init_req() with overlapping ranges and this could lead to an
assertion failure [1].

In particular, with the reproducer script below [0], two
cbw_co_snapshot_block_status() callers could race, with the second
calling reqlist_init_req() before the first one finishes and removes
its conflicting request.

[0]:

> #!/bin/bash -e
> dd if=/dev/urandom of=/tmp/disk.raw bs=1M count=1024
> ./qemu-img create /tmp/fleecing.raw -f raw 1G
> (
> ./qemu-system-x86_64 --qmp stdio \
> --blockdev raw,node-name=node0,file.driver=file,file.filename=/tmp/disk.raw \
> --blockdev raw,node-name=node1,file.driver=file,file.filename=/tmp/fleecing.raw \
> <<EOF
> {"execute": "qmp_capabilities"}
> {"execute": "blockdev-add", "arguments": { "driver": "copy-before-write", "file": "node0", "target": "node1", "node-name": "node3" } }
> {"execute": "blockdev-add", "arguments": { "driver": "snapshot-access", "file": "node3", "node-name": "snap0" } }
> {"execute": "nbd-server-start", "arguments": {"addr": { "type": "unix", "data": { "path": "/tmp/nbd.socket" } } } }
> {"execute": "block-export-add", "arguments": {"id": "exp0", "node-name": "snap0", "type": "nbd", "name": "exp0"}}
> EOF
> ) &
> sleep 5
> while true; do
> ./qemu-nbd -d /dev/nbd0
> ./qemu-nbd -c /dev/nbd0 nbd:unix:/tmp/nbd.socket:exportname=exp0 -f raw -r
> nbdinfo --map 'nbd+unix:///exp0?socket=/tmp/nbd.socket'
> done

[1]:

> Wind-River#5  0x000071e5f0088eb2 in __GI___assert_fail (...) at ./assert/assert.c:101
> Wind-River#6  0x0000615285438017 in reqlist_init_req (...) at ../block/reqlist.c:23
> Wind-River#7  0x00006152853e2d98 in cbw_snapshot_read_lock (...) at ../block/copy-before-write.c:237
> Wind-River#8  0x00006152853e3068 in cbw_co_snapshot_block_status (...) at ../block/copy-before-write.c:304
> Wind-River#9  0x00006152853f4d22 in bdrv_co_snapshot_block_status (...) at ../block/io.c:3726
> Wind-River#10 0x000061528543a63e in snapshot_access_co_block_status (...) at ../block/snapshot-access.c:48
> Wind-River#11 0x00006152853f1a0a in bdrv_co_do_block_status (...) at ../block/io.c:2474
> Wind-River#12 0x00006152853f2016 in bdrv_co_common_block_status_above (...) at ../block/io.c:2652
> Wind-River#13 0x00006152853f22cf in bdrv_co_block_status_above (...) at ../block/io.c:2732
> Wind-River#14 0x00006152853d9a86 in blk_co_block_status_above (...) at ../block/block-backend.c:1473
> Wind-River#15 0x000061528538da6c in blockstatus_to_extents (...) at ../nbd/server.c:2374
> Wind-River#16 0x000061528538deb1 in nbd_co_send_block_status (...) at ../nbd/server.c:2481
> Wind-River#17 0x000061528538f424 in nbd_handle_request (...) at ../nbd/server.c:2978
> Wind-River#18 0x000061528538f906 in nbd_trip (...) at ../nbd/server.c:3121
> Wind-River#19 0x00006152855a7caf in coroutine_trampoline (...) at ../util/coroutine-ucontext.c:175

Cc: [email protected]
Suggested-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Signed-off-by: Fiona Ebner <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
(cherry picked from commit 6475155)
Signed-off-by: Michael Tokarev <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Nov 15, 2024
@elmarco
vnc: fix crash when no console attached
5762cda 
Since commit e99441a ("ui/curses: Do not use console_select()")
qemu_text_console_put_keysym() no longer checks for NULL console
argument, which leads to a later crash:

Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
0x00005555559ee186 in qemu_text_console_handle_keysym (s=0x0, keysym=31) at ../ui/console-vc.c:332
332	        } else if (s->echo && (keysym == '\r' || keysym == '\n')) {
(gdb) bt
 #0  0x00005555559ee186 in qemu_text_console_handle_keysym (s=0x0, keysym=31) at ../ui/console-vc.c:332
 Wind-River#1  0x00005555559e18e5 in qemu_text_console_put_keysym (s=<optimized out>, keysym=<optimized out>) at ../ui/console.c:303
 Wind-River#2  0x00005555559f2e88 in do_key_event (vs=vs@entry=0x5555579045c0, down=down@entry=1, keycode=keycode@entry=60, sym=sym@entry=65471) at ../ui/vnc.c:2034
 Wind-River#3  0x00005555559f845c in ext_key_event (vs=0x5555579045c0, down=1, sym=65471, keycode=<optimized out>) at ../ui/vnc.c:2070
 Wind-River#4  protocol_client_msg (vs=0x5555579045c0, data=<optimized out>, len=<optimized out>) at ../ui/vnc.c:2514
 Wind-River#5  0x00005555559f515c in vnc_client_read (vs=0x5555579045c0) at ../ui/vnc.c:1607

Fixes: e99441a ("ui/curses: Do not use console_select()")
Fixes: https://issues.redhat.com/browse/RHEL-50529
Cc: [email protected]
Signed-off-by: Marc-André Lureau <[email protected]>
Reviewed-by: Akihiko Odaki <[email protected]>
Reviewed-by: Michael Tokarev <[email protected]>
Signed-off-by: Michael Tokarev <[email protected]>
(cherry picked from commit 0e60fc8)
Signed-off-by: Michael Tokarev <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Nov 15, 2024
@elmarco
hw/audio/hda: fix memory leak on audio setup
6d03242 
When SET_STREAM_FORMAT is called, we should clear the existing setup.

Factor out common function to close a stream.

Direct leak of 144 byte(s) in 3 object(s) allocated from:
    #0 0x7f91d38f7350 in calloc (/lib64/libasan.so.8+0xf7350) (BuildId: a4ad7eb954b390cf00f07fa10952988a41d9fc7a)
    Wind-River#1 0x7f91d2ab7871 in g_malloc0 (/lib64/libglib-2.0.so.0+0x64871) (BuildId: 36b60dbd02e796145a982d0151ce37202ec05649)
    Wind-River#2 0x562fa2f447ee in timer_new_full /home/elmarco/src/qemu/include/qemu/timer.h:538
    Wind-River#3 0x562fa2f4486f in timer_new /home/elmarco/src/qemu/include/qemu/timer.h:559
    Wind-River#4 0x562fa2f448a9 in timer_new_ns /home/elmarco/src/qemu/include/qemu/timer.h:577
    Wind-River#5 0x562fa2f47955 in hda_audio_setup ../hw/audio/hda-codec.c:490
    Wind-River#6 0x562fa2f4897e in hda_audio_command ../hw/audio/hda-codec.c:605

Signed-off-by: Marc-André Lureau <[email protected]>
Reviewed-by: Akihiko Odaki <[email protected]>
Message-ID: <[email protected]>
(cherry picked from commit 6d6e233)
Signed-off-by: Michael Tokarev <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
tests/qtest/migration-test: Don't leak resp in multifd_mapped_ram_fds…
0fa2cf8 
…et_end()

In multifd_mapped_ram_fdset_end() we call qtest_qmp() but forgot
to unref the response QDict we get back, which means it is leaked:

Indirect leak of 4120 byte(s) in 1 object(s) allocated from:
    #0 0x55c0c095d318 in __interceptor_calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/tests/qtest/migration-test+0x22f318) (BuildI
d: 07f667506452d6c467dbc06fd95191966d3e91b4)
    Wind-River#1 0x7f186f939c50 in g_malloc0 debian/build/deb/../../../glib/gmem.c:161:13
    Wind-River#2 0x55c0c0ae9b01 in qdict_new qobject/qdict.c:30:13
    Wind-River#3 0x55c0c0afc16c in parse_object qobject/json-parser.c:317:12
    Wind-River#4 0x55c0c0afb90f in parse_value qobject/json-parser.c:545:16
    Wind-River#5 0x55c0c0afb579 in json_parser_parse qobject/json-parser.c:579:14
    Wind-River#6 0x55c0c0afa21d in json_message_process_token qobject/json-streamer.c:92:12
    Wind-River#7 0x55c0c0bca2e5 in json_lexer_feed_char qobject/json-lexer.c:313:13
    Wind-River#8 0x55c0c0bc97ce in json_lexer_feed qobject/json-lexer.c:350:9
    Wind-River#9 0x55c0c0afabbc in json_message_parser_feed qobject/json-streamer.c:121:5
    Wind-River#10 0x55c0c09cbd52 in qmp_fd_receive tests/qtest/libqmp.c:86:9
    Wind-River#11 0x55c0c09be69b in qtest_qmp_receive_dict tests/qtest/libqtest.c:760:12
    Wind-River#12 0x55c0c09bca77 in qtest_qmp_receive tests/qtest/libqtest.c:741:27
    Wind-River#13 0x55c0c09bee9d in qtest_vqmp tests/qtest/libqtest.c:812:12
    Wind-River#14 0x55c0c09bd257 in qtest_qmp tests/qtest/libqtest.c:835:16
    Wind-River#15 0x55c0c0a87747 in multifd_mapped_ram_fdset_end tests/qtest/migration-test.c:2393:12
    Wind-River#16 0x55c0c0a85eb3 in test_file_common tests/qtest/migration-test.c:1978:9
    Wind-River#17 0x55c0c0a746a3 in test_multifd_file_mapped_ram_fdset tests/qtest/migration-test.c:2437:5
    Wind-River#18 0x55c0c0a93237 in migration_test_wrapper tests/qtest/migration-helpers.c:458:5
    Wind-River#19 0x7f186f958aed in test_case_run debian/build/deb/../../../glib/gtestutils.c:2930:15
    Wind-River#20 0x7f186f958aed in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3018:16
    Wind-River#21 0x7f186f95880a in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3035:18
    Wind-River#22 0x7f186f95880a in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3035:18
    Wind-River#23 0x7f186f95880a in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3035:18
    Wind-River#24 0x7f186f95880a in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3035:18
    Wind-River#25 0x7f186f95880a in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3035:18
    Wind-River#26 0x7f186f958faa in g_test_run_suite debian/build/deb/../../../glib/gtestutils.c:3109:18
    Wind-River#27 0x7f186f959055 in g_test_run debian/build/deb/../../../glib/gtestutils.c:2231:7
    #28 0x7f186f959055 in g_test_run debian/build/deb/../../../glib/gtestutils.c:2218:1
    #29 0x55c0c0a6e427 in main tests/qtest/migration-test.c:4033:11

Unref the object after we've confirmed that it is what we expect.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
tests/qtest/migration-test: Fix leaks in calc_dirtyrate_ready()
f0d7477 
In calc_dirtyrate_ready() we g_strdup() a string but then never free it:

Direct leak of 19 byte(s) in 2 object(s) allocated from:
    #0 0x55ead613413e in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/tests/qtest/migration-test+0x22f13e) (BuildId: e7cd5c37b2987a1af682b43ee5240b98bb316737)
    Wind-River#1 0x7f7a13d39738 in g_malloc debian/build/deb/../../../glib/gmem.c:128:13
    Wind-River#2 0x7f7a13d4e583 in g_strdup debian/build/deb/../../../glib/gstrfuncs.c:361:17
    Wind-River#3 0x55ead6266f48 in calc_dirtyrate_ready tests/qtest/migration-test.c:3409:14
    Wind-River#4 0x55ead62669fe in wait_for_calc_dirtyrate_complete tests/qtest/migration-test.c:3422:13
    Wind-River#5 0x55ead6253df7 in test_vcpu_dirty_limit tests/qtest/migration-test.c:3562:9
    Wind-River#6 0x55ead626a407 in migration_test_wrapper tests/qtest/migration-helpers.c:456:5

We also fail to unref the QMP rsp_return, so we leak that also.

Rather than duplicating the string, use the in-place value from
the qdict, and then unref the qdict.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
tests/qtest/migration-helpers: Fix migrate_get_socket_address() leak
c94170a 
In migrate_get_socket_address() we leak the SocketAddressList:
 (cd build/asan && \
  ASAN_OPTIONS="fast_unwind_on_malloc=0:strip_path_prefix=/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../"
  QTEST_QEMU_BINARY=./qemu-system-x86_64 \
  ./tests/qtest/migration-test --tap -k -p /x86_64/migration/multifd/tcp/tls/psk/match )

[...]
Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x563d7f22f318 in __interceptor_calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/tests/qtest/migration-test+0x22f318) (BuildId: 2ad6282fb5d076c863ab87f41a345d46dc965ded)
    Wind-River#1 0x7f9de3b39c50 in g_malloc0 debian/build/deb/../../../glib/gmem.c:161:13
    Wind-River#2 0x563d7f3a119c in qobject_input_start_list qapi/qobject-input-visitor.c:336:17
    Wind-River#3 0x563d7f390fbf in visit_start_list qapi/qapi-visit-core.c:80:10
    Wind-River#4 0x563d7f3882ef in visit_type_SocketAddressList /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qapi/qapi-visit-sockets.c:519:10
    Wind-River#5 0x563d7f3658c9 in migrate_get_socket_address tests/qtest/migration-helpers.c:97:5
    Wind-River#6 0x563d7f362e24 in migrate_get_connect_uri tests/qtest/migration-helpers.c:111:13
    Wind-River#7 0x563d7f362bb2 in migrate_qmp tests/qtest/migration-helpers.c:222:23
    Wind-River#8 0x563d7f3533cd in test_precopy_common tests/qtest/migration-test.c:1817:5
    Wind-River#9 0x563d7f34dc1c in test_multifd_tcp_tls_psk_match tests/qtest/migration-test.c:3185:5
    Wind-River#10 0x563d7f365337 in migration_test_wrapper tests/qtest/migration-helpers.c:458:5

The code fishes out the SocketAddress from the list to return it, and the
callers are freeing that, but nothing frees the list.

Since this function is called in only two places, the simple fix is to
make it return the SocketAddressList rather than just a SocketAddress,
and then the callers can easily access the SocketAddress, and free
the whole SocketAddressList when they're done.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
tests/qtest/migration-test: Free QCRyptoTLSTestCertReq objects
2cf6dc4 
In the migration test we create several TLS certificates with
the TLS_* macros from crypto-tls-x509-helpers.h. These macros
create both a QCryptoTLSCertReq object which must be deinitialized
and also an on-disk certificate file. The migration test currently
removes the on-disk file in test_migrate_tls_x509_finish() but
never deinitializes the QCryptoTLSCertReq, which means that memory
allocated as part of it is leaked:

Indirect leak of 2 byte(s) in 1 object(s) allocated from:
    #0 0x5558ba33712e in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/tests/qtest/migration-test+0x22f12e) (BuildId: 4c8618f663e538538cad19d35233124cea161491)
    Wind-River#1 0x7f64afc131f4  (/lib/x86_64-linux-gnu/libtasn1.so.6+0x81f4) (BuildId: 2fde6ecb43c586fe4077118f771077aa1298e7ea)
    Wind-River#2 0x7f64afc18d58 in asn1_write_value (/lib/x86_64-linux-gnu/libtasn1.so.6+0xdd58) (BuildId: 2fde6ecb43c586fe4077118f771077aa1298e7ea)
    Wind-River#3 0x7f64af8fc678 in gnutls_x509_crt_set_version (/lib/x86_64-linux-gnu/libgnutls.so.30+0xe7678) (BuildId: 97b8f99f392f1fd37b969a7164bcea884e23649b)
    Wind-River#4 0x5558ba470035 in test_tls_generate_cert tests/unit/crypto-tls-x509-helpers.c:234:5
    Wind-River#5 0x5558ba464e4a in test_migrate_tls_x509_start_common tests/qtest/migration-test.c:1058:5
    Wind-River#6 0x5558ba462c8a in test_migrate_tls_x509_start_default_host tests/qtest/migration-test.c:1123:12
    Wind-River#7 0x5558ba45ab40 in test_precopy_common tests/qtest/migration-test.c:1786:21
    Wind-River#8 0x5558ba450015 in test_precopy_unix_tls_x509_default_host tests/qtest/migration-test.c:2077:5
    Wind-River#9 0x5558ba46d3c7 in migration_test_wrapper tests/qtest/migration-helpers.c:456:5

(and similar reports).

The only function currently provided to deinit a QCryptoTLSCertReq is
test_tls_discard_cert(), which also removes the on-disk certificate
file.  For the migration tests we need to retain the on-disk files
until we've finished running the test, so the simplest fix is to
provide a new function test_tls_deinit_cert() which does only the
cleanup of the QCryptoTLSCertReq, and call it in the right places.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
tests/unit/crypto-tls-x509-helpers: deinit privkey in test_tls_cleanup
ba38592 
We create a gnutls_x509_privkey_t in test_tls_init(), but forget
to deinit it in test_tls_cleanup(), resulting in leaks
reported in hte migration test such as:

Indirect leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x55fa6d11c12e in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/tests/qtest/migration-test+0x22f12e) (BuildId: 852a267993587f557f50e5715f352f43720077ba)
    Wind-River#1 0x7f073982685d in __gmp_default_allocate (/lib/x86_64-linux-gnu/libgmp.so.10+0xa85d) (BuildId: f110719303ddbea25a5e89ff730fec520eed67b0)
    Wind-River#2 0x7f0739836193 in __gmpz_realloc (/lib/x86_64-linux-gnu/libgmp.so.10+0x1a193) (BuildId: f110719303ddbea25a5e89ff730fec520eed67b0)
    Wind-River#3 0x7f0739836594 in __gmpz_import (/lib/x86_64-linux-gnu/libgmp.so.10+0x1a594) (BuildId: f110719303ddbea25a5e89ff730fec520eed67b0)
    Wind-River#4 0x7f07398a91ed in nettle_mpz_set_str_256_u (/lib/x86_64-linux-gnu/libhogweed.so.6+0xb1ed) (BuildId: 3cc4a3474de72db89e9dcc93bfb95fe377f48c37)
    Wind-River#5 0x7f073a146a5a  (/lib/x86_64-linux-gnu/libgnutls.so.30+0x131a5a) (BuildId: 97b8f99f392f1fd37b969a7164bcea884e23649b)
    Wind-River#6 0x7f073a07192c  (/lib/x86_64-linux-gnu/libgnutls.so.30+0x5c92c) (BuildId: 97b8f99f392f1fd37b969a7164bcea884e23649b)
    Wind-River#7 0x7f073a078333  (/lib/x86_64-linux-gnu/libgnutls.so.30+0x63333) (BuildId: 97b8f99f392f1fd37b969a7164bcea884e23649b)
    Wind-River#8 0x7f073a0e8353  (/lib/x86_64-linux-gnu/libgnutls.so.30+0xd3353) (BuildId: 97b8f99f392f1fd37b969a7164bcea884e23649b)
    Wind-River#9 0x7f073a0ef0ac in gnutls_x509_privkey_import (/lib/x86_64-linux-gnu/libgnutls.so.30+0xda0ac) (BuildId: 97b8f99f392f1fd37b969a7164bcea884e23649b)
    Wind-River#10 0x55fa6d2547e3 in test_tls_load_key tests/unit/crypto-tls-x509-helpers.c:99:11
    Wind-River#11 0x55fa6d25460c in test_tls_init tests/unit/crypto-tls-x509-helpers.c:128:15
    Wind-River#12 0x55fa6d2495c4 in test_migrate_tls_x509_start_common tests/qtest/migration-test.c:1044:5
    Wind-River#13 0x55fa6d24c23a in test_migrate_tls_x509_start_reject_anon_client tests/qtest/migration-test.c:1216:12
    Wind-River#14 0x55fa6d23fb40 in test_precopy_common tests/qtest/migration-test.c:1789:21
    Wind-River#15 0x55fa6d236b7c in test_precopy_tcp_tls_x509_reject_anon_client tests/qtest/migration-test.c:2614:5

(Oddly, there is no reported leak in the x509 unit tests, even though
those also use test_tls_init() and test_tls_cleanup().)

Deinit the privkey in test_tls_cleanup().

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
tests/qtest/migration-helpers: Don't dup argument to qdict_put_str()
6ed8c95 
In migrate_set_ports() we call qdict_put_str() with a value string
which we g_strdup(). However qdict_put_str() takes a copy of the
value string, it doesn't take ownership of it, so the g_strdup()
only results in a leak:

Direct leak of 6 byte(s) in 1 object(s) allocated from:
    #0 0x56298023713e in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/tests/qtest/migration-test+0x22f13e) (BuildId: b2b9174a5a54707a7f76bca51cdc95d2aa08bac1)
    Wind-River#1 0x7fba0ad39738 in g_malloc debian/build/deb/../../../glib/gmem.c:128:13
    Wind-River#2 0x7fba0ad4e583 in g_strdup debian/build/deb/../../../glib/gstrfuncs.c:361:17
    Wind-River#3 0x56298036b16e in migrate_set_ports tests/qtest/migration-helpers.c:145:49
    Wind-River#4 0x56298036ad1c in migrate_qmp tests/qtest/migration-helpers.c:228:9
    Wind-River#5 0x56298035b3dd in test_precopy_common tests/qtest/migration-test.c:1820:5
    Wind-River#6 0x5629803549dc in test_multifd_tcp_channels_none tests/qtest/migration-test.c:3077:5
    Wind-River#7 0x56298036d427 in migration_test_wrapper tests/qtest/migration-helpers.c:456:5

Drop the unnecessary g_strdup() call.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
tests/qtest/migration-test: Don't strdup in get_dirty_rate()
78a053b 
We g_strdup() the "status" string we get out of the qdict in
get_dirty_rate(), but we never free it.  Since we only use this
string while the dictionary is still valid, we don't need to strdup
at all; drop the unnecessary call to avoid this leak:

Direct leak of 18 byte(s) in 2 object(s) allocated from:
    #0 0x564b3e01913e in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/tests/qtest/migration-test+0x22f13e) (BuildId: d6403a811332fcc846f93c45e23abfd06d1e67c4)
    Wind-River#1 0x7f2f278ff738 in g_malloc debian/build/deb/../../../glib/gmem.c:128:13
    Wind-River#2 0x7f2f27914583 in g_strdup debian/build/deb/../../../glib/gstrfuncs.c:361:17
    Wind-River#3 0x564b3e14bb5b in get_dirty_rate tests/qtest/migration-test.c:3447:14
    Wind-River#4 0x564b3e138e00 in test_vcpu_dirty_limit tests/qtest/migration-test.c:3565:16
    Wind-River#5 0x564b3e14f417 in migration_test_wrapper tests/qtest/migration-helpers.c:456:5

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
tests/qtest/migration-test: Don't leak QTestState in test_multifd_tcp…
854f67f 
…_cancel()

In test_multifd_tcp_cancel() we create three QEMU processes: 'from',
'to' and 'to2'.  We clean up (via qtest_quit()) 'from' and 'to2' when
we call test_migrate_end(), but never clean up 'to', which results in
this leak:

Direct leak of 336 byte(s) in 1 object(s) allocated from:
    #0 0x55e984fcd328 in __interceptor_calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/tests/qtest/migration-test+0x22f328) (BuildId: 710d409b68bb04427009e9ca6e1b63ff8af785d3)
    Wind-River#1 0x7f0878b39c50 in g_malloc0 debian/build/deb/../../../glib/gmem.c:161:13
    Wind-River#2 0x55e98503a172 in qtest_spawn_qemu tests/qtest/libqtest.c:397:21
    Wind-River#3 0x55e98502bc4a in qtest_init_internal tests/qtest/libqtest.c:471:9
    Wind-River#4 0x55e98502c5b7 in qtest_init_with_env tests/qtest/libqtest.c:533:21
    Wind-River#5 0x55e9850eef0f in test_migrate_start tests/qtest/migration-test.c:857:11
    Wind-River#6 0x55e9850eb01d in test_multifd_tcp_cancel tests/qtest/migration-test.c:3297:9
    Wind-River#7 0x55e985103407 in migration_test_wrapper tests/qtest/migration-helpers.c:456:5

Call qtest_quit() on 'to' to clean it up once it has exited.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Fabiano Rosas <[email protected]>
Signed-off-by: Fabiano Rosas <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
hw/misc/xlnx-versal-cfu: destroy fifo in finalize
d30d590 
Since the TYPE_XNLX_VERSAL_CFU_FDRO device creates a FIFO in its
instance_init method, we must destroy the FIFO in instance_finalize
to avoid a memory leak for the QOM introspection
"instantiate-examine-finalize" cycle:

Direct leak of 8192 byte(s) in 1 object(s) allocated from:
    #0 0x55ec89eae7ee in malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qemu-system-aarch64+0x294d7ee) (BuildId: 6d508874816cc47d17c8dd775e8f809ae520e8cb)
    Wind-River#1 0x7f697018f738 in g_malloc debian/build/deb/../../../glib/gmem.c:128:13
    Wind-River#2 0x55ec8d98d98d in fifo8_create util/fifo8.c:27:18
    Wind-River#3 0x55ec8aa2a624 in fifo32_create /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/include/qemu/fifo32.h:35:5
    Wind-River#4 0x55ec8aa2a33c in cfu_fdro_init hw/misc/xlnx-versal-cfu.c:397:5
    Wind-River#5 0x55ec8ce75da1 in object_init_with_type qom/object.c:420:9
    Wind-River#6 0x55ec8ce5d07b in object_initialize_with_type qom/object.c:562:5
    Wind-River#7 0x55ec8ce5e91d in object_new_with_type qom/object.c:782:5
    Wind-River#8 0x55ec8ce5e9f1 in object_new qom/object.c:797:12
    Wind-River#9 0x55ec8d65c81d in qmp_device_list_properties qom/qom-qmp-cmds.c:144:11

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Edgar E. Iglesias <[email protected]>
Reviewed-by: Francisco Iglesias <[email protected]>
Reviewed-by: Alistair Francis <[email protected]>
Message-id: [email protected]
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
hw/misc/xlnx-versal-trng: Free s->prng in finalize, not unrealize
abbfe8d 
The TYPE_XLNX_VERSAL_TRNG device creates s->prng with g_rand_new()
in its init method, but it frees it in its unrealize method. This
results in a leak in the QOM introspection "initialize-inspect-finalize"
lifecycle:

Direct leak of 2500 byte(s) in 1 object(s) allocated from:
    #0 0x55ec89eae9d8 in __interceptor_calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qemu-system-aarch64+0x294d9d8) (BuildId: 6d5
08874816cc47d17c8dd775e8f809ae520e8cb)
    Wind-River#1 0x7f697018fc50 in g_malloc0 debian/build/deb/../../../glib/gmem.c:161:13
    Wind-River#2 0x7f6970197738 in g_rand_new_with_seed_array debian/build/deb/../../../glib/grand.c:202:17
    Wind-River#3 0x7f6970197816 in g_rand_new debian/build/deb/../../../glib/grand.c:286:10
    Wind-River#4 0x55ec8aa3656a in trng_init hw/misc/xlnx-versal-trng.c:624:15
    Wind-River#5 0x55ec8ce75da1 in object_init_with_type qom/object.c:420:9
    Wind-River#6 0x55ec8ce5d07b in object_initialize_with_type qom/object.c:562:5
    Wind-River#7 0x55ec8ce5e91d in object_new_with_type qom/object.c:782:5
    Wind-River#8 0x55ec8ce5e9f1 in object_new qom/object.c:797:12
    Wind-River#9 0x55ec8d65c81d in qmp_device_list_properties qom/qom-qmp-cmds.c:144:11

Move the free to finalize so it matches where we are initing
s->prng. Since that's the only thing our unrealize method was
doing, this essentially switches the whole function to be
a finalize implementation.

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Edgar E. Iglesias <[email protected]>
Reviewed-by: Francisco Iglesias <[email protected]>
Reviewed-by: Alistair Francis <[email protected]>
Message-id: [email protected]
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
hw/nvram/xlnx-bbram: Call register_finalize_block
4a87373 
The TYPE_XLNX_BBRAM device creates a register block with
register_init_block32() in its instance_init method; we must
therefore destroy it in our instance_finalize method to avoid a leak
in the QOM introspection "init-inspect-finalize" lifecycle:

Direct leak of 304 byte(s) in 1 object(s) allocated from:
    #0 0x5641518ca9d8 in __interceptor_calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qemu-system-aarch64+0x294d9d8) (BuildId: 4a6
18cb63d57d5a19ed45cfc262b08da47eaafe5)
    Wind-River#1 0x7ff1aab31c50 in g_malloc0 debian/build/deb/../../../glib/gmem.c:161:13
    Wind-River#2 0x564151cffc5d in register_init_block hw/core/register.c:248:34
    Wind-River#3 0x564151d006be in register_init_block32 hw/core/register.c:299:12
    Wind-River#4 0x56415293df75 in bbram_ctrl_init hw/nvram/xlnx-bbram.c:462:9
    Wind-River#5 0x564154891dc1 in object_init_with_type qom/object.c:420:9
    Wind-River#6 0x56415487909b in object_initialize_with_type qom/object.c:562:5
    Wind-River#7 0x56415487a93d in object_new_with_type qom/object.c:782:5
    Wind-River#8 0x56415487aa11 in object_new qom/object.c:797:12
    Wind-River#9 0x56415507883d in qmp_device_list_properties qom/qom-qmp-cmds.c:144:11

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Edgar E. Iglesias <[email protected]>
Reviewed-by: Francisco Iglesias <[email protected]>
Reviewed-by: Alistair Francis <[email protected]>
Message-id: [email protected]
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
hw/nvram/xlnx-zynqmp-efuse: Call register_finalize_block
431d4d2 
The TYPE_XLNX_ZYNQMP_EFUSE device creates a register block with
register_init_block32() in its instance_init method; we must
therefore destroy it in our instance_finalize method to avoid a leak
in the QOM introspection "init-inspect-finalize" lifecycle:

Direct leak of 304 byte(s) in 1 object(s) allocated from:
    #0 0x55f3ff5839d8 in __interceptor_calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qemu-system-aarch64+0x294d9d8) (BuildId: 23cf931c66865a71b6cc4da95156d03bc106fa72)
    Wind-River#1 0x7f3f31c6bc50 in g_malloc0 debian/build/deb/../../../glib/gmem.c:161:13
    Wind-River#2 0x55f3ff9b8c5d in register_init_block hw/core/register.c:248:34
    Wind-River#3 0x55f3ff9b96be in register_init_block32 hw/core/register.c:299:12
    Wind-River#4 0x55f4005e5b25 in efuse_ctrl_init hw/nvram/xlnx-versal-efuse-ctrl.c:718:9
    Wind-River#5 0x55f40254afb1 in object_init_with_type qom/object.c:420:9
    Wind-River#6 0x55f40253228b in object_initialize_with_type qom/object.c:562:5
    Wind-River#7 0x55f402533b2d in object_new_with_type qom/object.c:782:5
    Wind-River#8 0x55f402533c01 in object_new qom/object.c:797:12
    Wind-River#9 0x55f402d31a2d in qmp_device_list_properties qom/qom-qmp-cmds.c:144:11

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Edgar E. Iglesias <[email protected]>
Reviewed-by: Francisco Iglesias <[email protected]>
Reviewed-by: Alistair Francis <[email protected]>
Message-id: [email protected]
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
hw/misc/xlnx-versal-trng: Call register_finalize_block
14bfca0 
The TYPE_XLNX_VERSAL_TRNG device creates a register block with
register_init_block32() in its instance_init method; we must
therefore destroy it in our instance_finalize method to avoid a leak
in the QOM introspection "init-inspect-finalize" lifecycle:

Direct leak of 304 byte(s) in 1 object(s) allocated from:
    #0 0x55842ec799d8 in __interceptor_calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qemu-system-aarch64+0x294e9d8) (BuildId: 47496e53f3e779f1c7e9b82cbea07407152b498b)
    Wind-River#1 0x7fe793c75c50 in g_malloc0 debian/build/deb/../../../glib/gmem.c:161:13
    Wind-River#2 0x55842f0aec5d in register_init_block hw/core/register.c:248:34
    Wind-River#3 0x55842f0af6be in register_init_block32 hw/core/register.c:299:12
    Wind-River#4 0x55842f801588 in trng_init hw/misc/xlnx-versal-trng.c:614:9
    Wind-River#5 0x558431c411a1 in object_init_with_type qom/object.c:420:9
    Wind-River#6 0x558431c2847b in object_initialize_with_type qom/object.c:562:5
    Wind-River#7 0x558431c29d1d in object_new_with_type qom/object.c:782:5
    Wind-River#8 0x558431c29df1 in object_new qom/object.c:797:12
    Wind-River#9 0x558432427c1d in qmp_device_list_properties qom/qom-qmp-cmds.c:144:11

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Edgar E. Iglesias <[email protected]>
Reviewed-by: Francisco Iglesias <[email protected]>
Reviewed-by: Alistair Francis <[email protected]>
Message-id: [email protected]
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@pm215
hm/nvram/xlnx-versal-efuse-ctrl: Call register_finalize_block
5b836d0 
The TYPE_XLNX_VERSAL_EFUSE_CTRL device creates a register block with
register_init_block32() in its instance_init method; we must
therefore destroy it in our instance_finalize method to avoid a leak
in the QOM introspection "init-inspect-finalize" lifecycle:

Direct leak of 304 byte(s) in 1 object(s) allocated from:
    #0 0x55f222b5b9d8 in __interceptor_calloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qemu-system-aarch64+0x294e9d8) (BuildId: 420
43d49e1139e3f3071b1f22fac1e3e7249c9a6)
    Wind-River#1 0x7fbb10669c50 in g_malloc0 debian/build/deb/../../../glib/gmem.c:161:13
    Wind-River#2 0x55f222f90c5d in register_init_block hw/core/register.c:248:34
    Wind-River#3 0x55f222f916be in register_init_block32 hw/core/register.c:299:12
    Wind-River#4 0x55f223bbdd15 in efuse_ctrl_init hw/nvram/xlnx-versal-efuse-ctrl.c:718:9
    Wind-River#5 0x55f225b23391 in object_init_with_type qom/object.c:420:9
    Wind-River#6 0x55f225b0a66b in object_initialize_with_type qom/object.c:562:5
    Wind-River#7 0x55f225b0bf0d in object_new_with_type qom/object.c:782:5
    Wind-River#8 0x55f225b0bfe1 in object_new qom/object.c:797:12
    Wind-River#9 0x55f226309e0d in qmp_device_list_properties qom/qom-qmp-cmds.c:144:11

Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Edgar E. Iglesias <[email protected]>
Reviewed-by: Francisco Iglesias <[email protected]>
Reviewed-by: Alistair Francis <[email protected]>
Message-id: [email protected]
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@foxmox
block/reqlist: allow adding overlapping requests
6475155 
Allow overlapping request by removing the assert that made it
impossible. There are only two callers:

1. block_copy_task_create()

It already asserts the very same condition before calling
reqlist_init_req().

2. cbw_snapshot_read_lock()

There is no need to have read requests be non-overlapping in
copy-before-write when used for snapshot-access. In fact, there was no
protection against two callers of cbw_snapshot_read_lock() calling
reqlist_init_req() with overlapping ranges and this could lead to an
assertion failure [1].

In particular, with the reproducer script below [0], two
cbw_co_snapshot_block_status() callers could race, with the second
calling reqlist_init_req() before the first one finishes and removes
its conflicting request.

[0]:

> #!/bin/bash -e
> dd if=/dev/urandom of=/tmp/disk.raw bs=1M count=1024
> ./qemu-img create /tmp/fleecing.raw -f raw 1G
> (
> ./qemu-system-x86_64 --qmp stdio \
> --blockdev raw,node-name=node0,file.driver=file,file.filename=/tmp/disk.raw \
> --blockdev raw,node-name=node1,file.driver=file,file.filename=/tmp/fleecing.raw \
> <<EOF
> {"execute": "qmp_capabilities"}
> {"execute": "blockdev-add", "arguments": { "driver": "copy-before-write", "file": "node0", "target": "node1", "node-name": "node3" } }
> {"execute": "blockdev-add", "arguments": { "driver": "snapshot-access", "file": "node3", "node-name": "snap0" } }
> {"execute": "nbd-server-start", "arguments": {"addr": { "type": "unix", "data": { "path": "/tmp/nbd.socket" } } } }
> {"execute": "block-export-add", "arguments": {"id": "exp0", "node-name": "snap0", "type": "nbd", "name": "exp0"}}
> EOF
> ) &
> sleep 5
> while true; do
> ./qemu-nbd -d /dev/nbd0
> ./qemu-nbd -c /dev/nbd0 nbd:unix:/tmp/nbd.socket:exportname=exp0 -f raw -r
> nbdinfo --map 'nbd+unix:///exp0?socket=/tmp/nbd.socket'
> done

[1]:

> Wind-River#5  0x000071e5f0088eb2 in __GI___assert_fail (...) at ./assert/assert.c:101
> Wind-River#6  0x0000615285438017 in reqlist_init_req (...) at ../block/reqlist.c:23
> Wind-River#7  0x00006152853e2d98 in cbw_snapshot_read_lock (...) at ../block/copy-before-write.c:237
> Wind-River#8  0x00006152853e3068 in cbw_co_snapshot_block_status (...) at ../block/copy-before-write.c:304
> Wind-River#9  0x00006152853f4d22 in bdrv_co_snapshot_block_status (...) at ../block/io.c:3726
> Wind-River#10 0x000061528543a63e in snapshot_access_co_block_status (...) at ../block/snapshot-access.c:48
> Wind-River#11 0x00006152853f1a0a in bdrv_co_do_block_status (...) at ../block/io.c:2474
> Wind-River#12 0x00006152853f2016 in bdrv_co_common_block_status_above (...) at ../block/io.c:2652
> Wind-River#13 0x00006152853f22cf in bdrv_co_block_status_above (...) at ../block/io.c:2732
> Wind-River#14 0x00006152853d9a86 in blk_co_block_status_above (...) at ../block/block-backend.c:1473
> Wind-River#15 0x000061528538da6c in blockstatus_to_extents (...) at ../nbd/server.c:2374
> Wind-River#16 0x000061528538deb1 in nbd_co_send_block_status (...) at ../nbd/server.c:2481
> Wind-River#17 0x000061528538f424 in nbd_handle_request (...) at ../nbd/server.c:2978
> Wind-River#18 0x000061528538f906 in nbd_trip (...) at ../nbd/server.c:3121
> Wind-River#19 0x00006152855a7caf in coroutine_trampoline (...) at ../util/coroutine-ucontext.c:175

Cc: [email protected]
Suggested-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Signed-off-by: Fiona Ebner <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@elmarco
vnc: fix crash when no console attached
0e60fc8 
Since commit e99441a ("ui/curses: Do not use console_select()")
qemu_text_console_put_keysym() no longer checks for NULL console
argument, which leads to a later crash:

Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
0x00005555559ee186 in qemu_text_console_handle_keysym (s=0x0, keysym=31) at ../ui/console-vc.c:332
332	        } else if (s->echo && (keysym == '\r' || keysym == '\n')) {
(gdb) bt
 #0  0x00005555559ee186 in qemu_text_console_handle_keysym (s=0x0, keysym=31) at ../ui/console-vc.c:332
 Wind-River#1  0x00005555559e18e5 in qemu_text_console_put_keysym (s=<optimized out>, keysym=<optimized out>) at ../ui/console.c:303
 Wind-River#2  0x00005555559f2e88 in do_key_event (vs=vs@entry=0x5555579045c0, down=down@entry=1, keycode=keycode@entry=60, sym=sym@entry=65471) at ../ui/vnc.c:2034
 Wind-River#3  0x00005555559f845c in ext_key_event (vs=0x5555579045c0, down=1, sym=65471, keycode=<optimized out>) at ../ui/vnc.c:2070
 Wind-River#4  protocol_client_msg (vs=0x5555579045c0, data=<optimized out>, len=<optimized out>) at ../ui/vnc.c:2514
 Wind-River#5  0x00005555559f515c in vnc_client_read (vs=0x5555579045c0) at ../ui/vnc.c:1607

Fixes: e99441a ("ui/curses: Do not use console_select()")
Fixes: https://issues.redhat.com/browse/RHEL-50529
Cc: [email protected]
Signed-off-by: Marc-André Lureau <[email protected]>
Reviewed-by: Akihiko Odaki <[email protected]>
Reviewed-by: Michael Tokarev <[email protected]>
Signed-off-by: Michael Tokarev <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@elmarco
hw/audio/hda: fix memory leak on audio setup
6d6e233 
When SET_STREAM_FORMAT is called, we should clear the existing setup.

Factor out common function to close a stream.

Direct leak of 144 byte(s) in 3 object(s) allocated from:
    #0 0x7f91d38f7350 in calloc (/lib64/libasan.so.8+0xf7350) (BuildId: a4ad7eb954b390cf00f07fa10952988a41d9fc7a)
    Wind-River#1 0x7f91d2ab7871 in g_malloc0 (/lib64/libglib-2.0.so.0+0x64871) (BuildId: 36b60dbd02e796145a982d0151ce37202ec05649)
    Wind-River#2 0x562fa2f447ee in timer_new_full /home/elmarco/src/qemu/include/qemu/timer.h:538
    Wind-River#3 0x562fa2f4486f in timer_new /home/elmarco/src/qemu/include/qemu/timer.h:559
    Wind-River#4 0x562fa2f448a9 in timer_new_ns /home/elmarco/src/qemu/include/qemu/timer.h:577
    Wind-River#5 0x562fa2f47955 in hda_audio_setup ../hw/audio/hda-codec.c:490
    Wind-River#6 0x562fa2f4897e in hda_audio_command ../hw/audio/hda-codec.c:605

Signed-off-by: Marc-André Lureau <[email protected]>
Reviewed-by: Akihiko Odaki <[email protected]>
Message-ID: <[email protected]>
ho28 pushed a commit to ho28/wr-qemu that referenced this issue Jan 15, 2025
@snimmagadda @kostyanf14
qemu-ga: Fix a SIGSEGV in ga_run_command() helper
9cfe110 
qemu-ga on a NetBSD -current VM terminates with a SIGSEGV upon receiving
'guest-set-time' command...

Core was generated by `qemu-ga'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x000000000cd37a40 in ga_pipe_read_str (fd=fd@entry=0xffffff922a20, str=str@entry=0xffffff922a18)
    at ../qga/commands-posix.c:88
88	        *str[len] = '\0';
[Current thread is 1 (process 1112)]
(gdb) bt
#0  0x000000000cd37a40 in ga_pipe_read_str (fd=fd@entry=0xffffff922a20, str=str@entry=0xffffff922a18)
    at ../qga/commands-posix.c:88
Wind-River#1  0x000000000cd37b60 in ga_run_command (argv=argv@entry=0xffffff922a90,
    action=action@entry=0xcda34b8 "set hardware clock to system time", errp=errp@entry=0xffffff922a70, in_str=0x0)
    at ../qga/commands-posix.c:164
Wind-River#2  0x000000000cd380c4 in qmp_guest_set_time (has_time=<optimized out>, time_ns=<optimized out>,
    errp=errp@entry=0xffffff922ad0) at ../qga/commands-posix.c:304
Wind-River#3  0x000000000cd253d8 in qmp_marshal_guest_set_time (args=<optimized out>, ret=<optimized out>, errp=0xffffff922b48)
    at qga/qga-qapi-commands.c:193
Wind-River#4  0x000000000cd4e71c in qmp_dispatch (cmds=cmds@entry=0xcdf5b18 <ga_commands>, request=request@entry=0xf3c711a4b000,
    allow_oob=allow_oob@entry=false, cur_mon=cur_mon@entry=0x0) at ../qapi/qmp-dispatch.c:220
Wind-River#5  0x000000000cd36524 in process_event (opaque=0xf3c711a79000, obj=0xf3c711a4b000, err=0x0) at ../qga/main.c:677
Wind-River#6  0x000000000cd526f0 in json_message_process_token (lexer=lexer@entry=0xf3c711a79018, input=0xf3c712072480,
    type=type@entry=JSON_RCURLY, x=28, y=1) at ../qobject/json-streamer.c:99
Wind-River#7  0x000000000cd93860 in json_lexer_feed_char (lexer=lexer@entry=0xf3c711a79018, ch=125 '}', flush=flush@entry=false)
    at ../qobject/json-lexer.c:313
Wind-River#8  0x000000000cd93a00 in json_lexer_feed (lexer=lexer@entry=0xf3c711a79018,
    buffer=buffer@entry=0xffffff922d10 "{\"execute\":\"guest-set-time\"}\n", size=<optimized out>)
    at ../qobject/json-lexer.c:350
Wind-River#9  0x000000000cd5290c in json_message_parser_feed (parser=parser@entry=0xf3c711a79000,
    buffer=buffer@entry=0xffffff922d10 "{\"execute\":\"guest-set-time\"}\n", size=<optimized out>)
    at ../qobject/json-streamer.c:121
Wind-River#10 0x000000000cd361fc in channel_event_cb (condition=<optimized out>, data=0xf3c711a79000) at ../qga/main.c:703
Wind-River#11 0x000000000cd3710c in ga_channel_client_event (channel=<optimized out>, condition=<optimized out>, data=0xf3c711b2d300)
    at ../qga/channel-posix.c:94
Wind-River#12 0x0000f3c7120d9bec in g_main_dispatch () from /usr/pkg/lib/libglib-2.0.so.0
Wind-River#13 0x0000f3c7120dd25c in g_main_context_iterate_unlocked.constprop () from /usr/pkg/lib/libglib-2.0.so.0
Wind-River#14 0x0000f3c7120ddbf0 in g_main_loop_run () from /usr/pkg/lib/libglib-2.0.so.0
Wind-River#15 0x000000000cda00d8 in run_agent_once (s=0xf3c711a79000) at ../qga/main.c:1522
Wind-River#16 run_agent (s=0xf3c711a79000) at ../qga/main.c:1559
Wind-River#17 main (argc=<optimized out>, argv=<optimized out>) at ../qga/main.c:1671
(gdb)

The commandline options used on the host machine...
qemu-system-aarch64 \
   -machine type=virt,pflash0=rom \
   -m 8G \
   -cpu host \
   -smp 8 \
   -accel hvf \
   -device virtio-net-pci,netdev=unet \
   -device virtio-blk-pci,drive=hd \
   -drive file=netbsd.qcow2,if=none,id=hd \
   -netdev user,id=unet,hostfwd=tcp::2223-:22 \
   -object rng-random,filename=/dev/urandom,id=viornd0 \
   -device virtio-rng-pci,rng=viornd0 \
   -serial mon:stdio \
   -display none \
   -blockdev node-name=rom,driver=file,filename=/opt/homebrew/Cellar/qemu/9.0.2/share/qemu/edk2-aarch64-code.fd,read-only=true \
   -chardev socket,path=/tmp/qga_netbsd.sock,server=on,wait=off,id=qga0 \
   -device virtio-serial \
   -device virtconsole,chardev=qga0,name=org.qemu.guest_agent.0

This patch rectifies the operator precedence while assigning the NUL
terminator.

Fixes: c3f32c1

Signed-off-by: Sunil Nimmagadda <[email protected]>
Reviewed-by: Konstantin Kostiuk <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Konstantin Kostiuk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@ho28