src/bhyve/bhyve_command.c | 3 +- src/bhyve/bhyve_parse_command.c | 22 +-- src/libxl/libxl_conf.c | 9 +- src/libxl/xen_common.c | 18 +- src/libxl/xen_xl.c | 17 +- src/lxc/lxc_container.c | 4 +- src/lxc/lxc_native.c | 24 +-- src/qemu/qemu_driver.c | 17 +- src/remote/remote_daemon_dispatch.c | 3 +- src/remote/remote_driver.c | 4 +- src/storage/storage_backend_rbd.c | 3 +- src/util/vircgroup.c | 3 +- src/util/vircgroupv2.c | 4 +- src/util/virfirmware.c | 6 +- src/util/viruri.c | 3 +- src/vbox/vbox_common.c | 12 +- src/vbox/vbox_snapshot_conf.c | 40 ++-- src/vbox/vbox_tmpl.c | 3 +- src/vz/vz_sdk.c | 3 +- tests/qemuagenttest.c | 286 ++++++++++++---------------- tests/qemucapabilitiestest.c | 22 +-- tests/qemuhotplugtest.c | 3 +- tests/qemumigparamstest.c | 40 ++-- tests/qemumonitorjsontest.c | 95 ++++----- tests/qemumonitortestutils.c | 63 +++--- tests/vboxsnapshotxmltest.c | 3 +- tests/virconftest.c | 3 +- tests/virfiletest.c | 3 +- tests/virstringtest.c | 3 +- tools/virsh-host.c | 13 +- tools/virt-login-shell-helper.c | 7 +- tools/vsh.c | 4 +- 32 files changed, 279 insertions(+), 464 deletions(-)
I've been looking at our tests lately and noticed an opportunity to rewrite pieces of code to g_auto() magic. Michal Prívozník (7): qemuagenttest: Don't leak virTypedParameter on failure Prefer g_auto(GStrv) over g_strfreev() qemu: Use g_autoptr(qemuMonitorCPUModelInfo) qemuConnectStealCPUModelFromInfo: Drop needless 'cleanup' label tests: Use g_autoptr(qemuMonitorTest) test: Use g_autofree more tests: Drop cleanup/error labels src/bhyve/bhyve_command.c | 3 +- src/bhyve/bhyve_parse_command.c | 22 +-- src/libxl/libxl_conf.c | 9 +- src/libxl/xen_common.c | 18 +- src/libxl/xen_xl.c | 17 +- src/lxc/lxc_container.c | 4 +- src/lxc/lxc_native.c | 24 +-- src/qemu/qemu_driver.c | 17 +- src/remote/remote_daemon_dispatch.c | 3 +- src/remote/remote_driver.c | 4 +- src/storage/storage_backend_rbd.c | 3 +- src/util/vircgroup.c | 3 +- src/util/vircgroupv2.c | 4 +- src/util/virfirmware.c | 6 +- src/util/viruri.c | 3 +- src/vbox/vbox_common.c | 12 +- src/vbox/vbox_snapshot_conf.c | 40 ++-- src/vbox/vbox_tmpl.c | 3 +- src/vz/vz_sdk.c | 3 +- tests/qemuagenttest.c | 286 ++++++++++++---------------- tests/qemucapabilitiestest.c | 22 +-- tests/qemuhotplugtest.c | 3 +- tests/qemumigparamstest.c | 40 ++-- tests/qemumonitorjsontest.c | 95 ++++----- tests/qemumonitortestutils.c | 63 +++--- tests/vboxsnapshotxmltest.c | 3 +- tests/virconftest.c | 3 +- tests/virfiletest.c | 3 +- tests/virstringtest.c | 3 +- tools/virsh-host.c | 13 +- tools/virt-login-shell-helper.c | 7 +- tools/vsh.c | 4 +- 32 files changed, 279 insertions(+), 464 deletions(-) -- 2.32.0
On Mon, 2021-11-01 at 15:16 +0100, Michal Privoznik wrote: > I've been looking at our tests lately and noticed an opportunity to > rewrite pieces of code to g_auto() magic. > > Michal Prívozník (7): > qemuagenttest: Don't leak virTypedParameter on failure > Prefer g_auto(GStrv) over g_strfreev() > qemu: Use g_autoptr(qemuMonitorCPUModelInfo) > qemuConnectStealCPUModelFromInfo: Drop needless 'cleanup' label > tests: Use g_autoptr(qemuMonitorTest) > test: Use g_autofree more > tests: Drop cleanup/error labels > > src/bhyve/bhyve_command.c | 3 +- > src/bhyve/bhyve_parse_command.c | 22 +-- > src/libxl/libxl_conf.c | 9 +- > src/libxl/xen_common.c | 18 +- > src/libxl/xen_xl.c | 17 +- > src/lxc/lxc_container.c | 4 +- > src/lxc/lxc_native.c | 24 +-- > src/qemu/qemu_driver.c | 17 +- > src/remote/remote_daemon_dispatch.c | 3 +- > src/remote/remote_driver.c | 4 +- > src/storage/storage_backend_rbd.c | 3 +- > src/util/vircgroup.c | 3 +- > src/util/vircgroupv2.c | 4 +- > src/util/virfirmware.c | 6 +- > src/util/viruri.c | 3 +- > src/vbox/vbox_common.c | 12 +- > src/vbox/vbox_snapshot_conf.c | 40 ++-- > src/vbox/vbox_tmpl.c | 3 +- > src/vz/vz_sdk.c | 3 +- > tests/qemuagenttest.c | 286 ++++++++++++---------------- > tests/qemucapabilitiestest.c | 22 +-- > tests/qemuhotplugtest.c | 3 +- > tests/qemumigparamstest.c | 40 ++-- > tests/qemumonitorjsontest.c | 95 ++++----- > tests/qemumonitortestutils.c | 63 +++--- > tests/vboxsnapshotxmltest.c | 3 +- > tests/virconftest.c | 3 +- > tests/virfiletest.c | 3 +- > tests/virstringtest.c | 3 +- > tools/virsh-host.c | 13 +- > tools/virt-login-shell-helper.c | 7 +- > tools/vsh.c | 4 +- > 32 files changed, 279 insertions(+), 464 deletions(-) > When applying this series, compiling with ASAN enabled, and running "virsh hypervisor-cpu-compare empty.xml" with "empty.xml" == "<cpu/>", I see the following error message: ================================================================= ==45506==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000009b70 at pc 0x5588d1c81aa8 bp 0x7fffc8510af0 sp 0x7fffc8510ae8 READ of size 8 at 0x602000009b70 thread T0 #0 0x5588d1c81aa7 in cmdHypervisorCPUCompare ../../git/libvirt/tools/virsh-host.c:1605 #1 0x5588d1cead5d in vshCommandRun ../../git/libvirt/tools/vsh.c:1309 #2 0x5588d1bd5331 in main ../../git/libvirt/tools/virsh.c:899 #3 0x7fc8c4f32b74 in __libc_start_main (/lib64/libc.so.6+0x27b74) #4 0x5588d1bcef3d in _start (/home/twiederh/build/libvirt/tools/virsh+0x16bf3d) 0x602000009b70 is located 0 bytes inside of 16-byte region [0x602000009b70,0x602000009b80) freed by thread T0 here: #0 0x7fc8c9020647 in free (/lib64/libasan.so.6+0xae647) #1 0x7fc8c5b3a24c in g_free (/lib64/libglib-2.0.so.0+0x5a24c) #2 0x5588d1c7ebcb in vshExtractCPUDefXMLs ../../git/libvirt/tools/virsh-host.c:1062 #3 0x5588d1c819fe in cmdHypervisorCPUCompare ../../git/libvirt/tools/virsh-host.c:1602 #4 0x5588d1cead5d in vshCommandRun ../../git/libvirt/tools/vsh.c:1309 #5 0x5588d1bd5331 in main ../../git/libvirt/tools/virsh.c:899 #6 0x7fc8c4f32b74 in __libc_start_main (/lib64/libc.so.6+0x27b74) previously allocated by thread T0 here: #0 0x7fc8c9020af7 in calloc (/lib64/libasan.so.6+0xaeaf7) #1 0x7fc8c5b3de60 in g_malloc0 (/lib64/libglib-2.0.so.0+0x5de60) #2 0x5588d1c819fe in cmdHypervisorCPUCompare ../../git/libvirt/tools/virsh-host.c:1602 #3 0x5588d1cead5d in vshCommandRun ../../git/libvirt/tools/vsh.c:1309 #4 0x5588d1bd5331 in main ../../git/libvirt/tools/virsh.c:899 #5 0x7fc8c4f32b74 in __libc_start_main (/lib64/libc.so.6+0x27b74) SUMMARY: AddressSanitizer: heap-use-after-free ../../git/libvirt/tools/virsh-host.c:1605 in cmdHypervisorCPUCompare Shadow bytes around the buggy address: 0x0c047fff9310: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fa 0x0c047fff9320: fa fa fd fa fa fa fd fd fa fa fd fd fa fa fd fd 0x0c047fff9330: fa fa fd fa fa fa fd fa fa fa fd fd fa fa fd fd 0x0c047fff9340: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd 0x0c047fff9350: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd =>0x0c047fff9360: fa fa fd fd fa fa fd fd fa fa fd fa fa fa[fd]fd 0x0c047fff9370: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff9380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff9390: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff93a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff93b0: 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 Shadow gap: cc ==45506==ABORTING
On 11/9/21 3:16 PM, Tim Wiederhake wrote: > On Mon, 2021-11-01 at 15:16 +0100, Michal Privoznik wrote: >> I've been looking at our tests lately and noticed an opportunity to >> rewrite pieces of code to g_auto() magic. >> >> Michal Prívozník (7): >> qemuagenttest: Don't leak virTypedParameter on failure >> Prefer g_auto(GStrv) over g_strfreev() >> qemu: Use g_autoptr(qemuMonitorCPUModelInfo) >> qemuConnectStealCPUModelFromInfo: Drop needless 'cleanup' label >> tests: Use g_autoptr(qemuMonitorTest) >> test: Use g_autofree more >> tests: Drop cleanup/error labels >> >> src/bhyve/bhyve_command.c | 3 +- >> src/bhyve/bhyve_parse_command.c | 22 +-- >> src/libxl/libxl_conf.c | 9 +- >> src/libxl/xen_common.c | 18 +- >> src/libxl/xen_xl.c | 17 +- >> src/lxc/lxc_container.c | 4 +- >> src/lxc/lxc_native.c | 24 +-- >> src/qemu/qemu_driver.c | 17 +- >> src/remote/remote_daemon_dispatch.c | 3 +- >> src/remote/remote_driver.c | 4 +- >> src/storage/storage_backend_rbd.c | 3 +- >> src/util/vircgroup.c | 3 +- >> src/util/vircgroupv2.c | 4 +- >> src/util/virfirmware.c | 6 +- >> src/util/viruri.c | 3 +- >> src/vbox/vbox_common.c | 12 +- >> src/vbox/vbox_snapshot_conf.c | 40 ++-- >> src/vbox/vbox_tmpl.c | 3 +- >> src/vz/vz_sdk.c | 3 +- >> tests/qemuagenttest.c | 286 ++++++++++++---------------- >> tests/qemucapabilitiestest.c | 22 +-- >> tests/qemuhotplugtest.c | 3 +- >> tests/qemumigparamstest.c | 40 ++-- >> tests/qemumonitorjsontest.c | 95 ++++----- >> tests/qemumonitortestutils.c | 63 +++--- >> tests/vboxsnapshotxmltest.c | 3 +- >> tests/virconftest.c | 3 +- >> tests/virfiletest.c | 3 +- >> tests/virstringtest.c | 3 +- >> tools/virsh-host.c | 13 +- >> tools/virt-login-shell-helper.c | 7 +- >> tools/vsh.c | 4 +- >> 32 files changed, 279 insertions(+), 464 deletions(-) >> > > When applying this series, compiling with ASAN enabled, and running > "virsh hypervisor-cpu-compare empty.xml" with "empty.xml" == "<cpu/>", > I see the following error message: > > ================================================================= > ==45506==ERROR: AddressSanitizer: heap-use-after-free on address > 0x602000009b70 at pc 0x5588d1c81aa8 bp 0x7fffc8510af0 sp 0x7fffc8510ae8 > READ of size 8 at 0x602000009b70 thread T0 > #0 0x5588d1c81aa7 in cmdHypervisorCPUCompare > ../../git/libvirt/tools/virsh-host.c:1605 > #1 0x5588d1cead5d in vshCommandRun > ../../git/libvirt/tools/vsh.c:1309 > #2 0x5588d1bd5331 in main ../../git/libvirt/tools/virsh.c:899 > #3 0x7fc8c4f32b74 in __libc_start_main (/lib64/libc.so.6+0x27b74) > #4 0x5588d1bcef3d in _start > (/home/twiederh/build/libvirt/tools/virsh+0x16bf3d) Ah thanks, it's a problem in 2/7 where this needs to be squashed in: diff --git i/tools/virsh-host.c w/tools/virsh-host.c index f6aa532b40..fc84415e7b 100644 --- i/tools/virsh-host.c +++ w/tools/virsh-host.c @@ -1123,7 +1123,7 @@ vshExtractCPUDefXMLs(vshControl *ctl, } cleanup: - return cpus; + return g_steal_pointer(&cpus); error: goto cleanup; Michal
On Tue, 2021-11-09 at 16:24 +0100, Michal Prívozník wrote: > On 11/9/21 3:16 PM, Tim Wiederhake wrote: > > On Mon, 2021-11-01 at 15:16 +0100, Michal Privoznik wrote: > > > I've been looking at our tests lately and noticed an opportunity > > > to > > > rewrite pieces of code to g_auto() magic. > > > > > > Michal Prívozník (7): > > > qemuagenttest: Don't leak virTypedParameter on failure > > > Prefer g_auto(GStrv) over g_strfreev() > > > qemu: Use g_autoptr(qemuMonitorCPUModelInfo) > > > qemuConnectStealCPUModelFromInfo: Drop needless 'cleanup' label > > > tests: Use g_autoptr(qemuMonitorTest) > > > test: Use g_autofree more > > > tests: Drop cleanup/error labels > > > > > > src/bhyve/bhyve_command.c | 3 +- > > > src/bhyve/bhyve_parse_command.c | 22 +-- > > > src/libxl/libxl_conf.c | 9 +- > > > src/libxl/xen_common.c | 18 +- > > > src/libxl/xen_xl.c | 17 +- > > > src/lxc/lxc_container.c | 4 +- > > > src/lxc/lxc_native.c | 24 +-- > > > src/qemu/qemu_driver.c | 17 +- > > > src/remote/remote_daemon_dispatch.c | 3 +- > > > src/remote/remote_driver.c | 4 +- > > > src/storage/storage_backend_rbd.c | 3 +- > > > src/util/vircgroup.c | 3 +- > > > src/util/vircgroupv2.c | 4 +- > > > src/util/virfirmware.c | 6 +- > > > src/util/viruri.c | 3 +- > > > src/vbox/vbox_common.c | 12 +- > > > src/vbox/vbox_snapshot_conf.c | 40 ++-- > > > src/vbox/vbox_tmpl.c | 3 +- > > > src/vz/vz_sdk.c | 3 +- > > > tests/qemuagenttest.c | 286 ++++++++++++---------- > > > ------ > > > tests/qemucapabilitiestest.c | 22 +-- > > > tests/qemuhotplugtest.c | 3 +- > > > tests/qemumigparamstest.c | 40 ++-- > > > tests/qemumonitorjsontest.c | 95 ++++----- > > > tests/qemumonitortestutils.c | 63 +++--- > > > tests/vboxsnapshotxmltest.c | 3 +- > > > tests/virconftest.c | 3 +- > > > tests/virfiletest.c | 3 +- > > > tests/virstringtest.c | 3 +- > > > tools/virsh-host.c | 13 +- > > > tools/virt-login-shell-helper.c | 7 +- > > > tools/vsh.c | 4 +- > > > 32 files changed, 279 insertions(+), 464 deletions(-) > > > > > > > When applying this series, compiling with ASAN enabled, and running > > "virsh hypervisor-cpu-compare empty.xml" with "empty.xml" == > > "<cpu/>", > > I see the following error message: > > > > ================================================================= > > ==45506==ERROR: AddressSanitizer: heap-use-after-free on address > > 0x602000009b70 at pc 0x5588d1c81aa8 bp 0x7fffc8510af0 sp > > 0x7fffc8510ae8 > > READ of size 8 at 0x602000009b70 thread T0 > > #0 0x5588d1c81aa7 in cmdHypervisorCPUCompare > > ../../git/libvirt/tools/virsh-host.c:1605 > > #1 0x5588d1cead5d in vshCommandRun > > ../../git/libvirt/tools/vsh.c:1309 > > #2 0x5588d1bd5331 in main ../../git/libvirt/tools/virsh.c:899 > > #3 0x7fc8c4f32b74 in __libc_start_main > > (/lib64/libc.so.6+0x27b74) > > #4 0x5588d1bcef3d in _start > > (/home/twiederh/build/libvirt/tools/virsh+0x16bf3d) > > > Ah thanks, it's a problem in 2/7 where this needs to be squashed in: > > diff --git i/tools/virsh-host.c w/tools/virsh-host.c > index f6aa532b40..fc84415e7b 100644 > --- i/tools/virsh-host.c > +++ w/tools/virsh-host.c > @@ -1123,7 +1123,7 @@ vshExtractCPUDefXMLs(vshControl *ctl, > } > > cleanup: > - return cpus; > + return g_steal_pointer(&cpus); > > error: > goto cleanup; > > > Michal > With that applied: Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
© 2016 - 2024 Red Hat, Inc.