[PATCH v2 0/9] Various ASAN & tests fixes

[PATCH v2 0/9] Various ASAN & tests fixes

[Marc-André Lureau]

Hi,

A collection of leak-related fixes.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
Changes in v2:
- add "tests/exynos4210_uart_fifo-test: fix buffer leaks"
- add "tests/xilinx-zynq-sdhci-test: fix GIOChannel leak"
- add "glib-compat: fix g_autofd fallback warnings"
- rebased, collect trailer tags
- Link to v1: https://lore.kernel.org/qemu-devel/20260906-nohmp-next-v1-0-2b21e63974f3@redhat.com

---
Marc-André Lureau (9):
      tests/launchupdate-test: correct g_auto usage
      tests/launchupdate-test: correctly release qs
      tests/launchupdate-test: fix fw_cfg leak
      igvm: release found memory region on success
      igvm: make IgvmMemoryRegion a QOM
      hw/s390x/ipl: fix short SCSI loadparm buffer over-read
      glib-compat: fix g_autofd fallback warnings
      tests/exynos4210_uart_fifo-test: fix buffer leaks
      tests/xilinx-zynq-sdhci-test: fix GIOChannel leak

 backends/igvm.c                         | 99 ++++++++++++++++++++-------------
 hw/s390x/ipl.c                          | 10 ++--
 include/glib-compat.h                   |  8 +--
 include/system/igvm-internal.h          |  9 +--
 tests/qtest/exynos4210_uart_fifo-test.c |  6 +-
 tests/qtest/launchupdate-test.c         | 20 +++----
 tests/qtest/libqos/libqos.h             |  2 +
 tests/qtest/xilinx-zynq-sdhci-test.c    |  6 +-
 8 files changed, 88 insertions(+), 72 deletions(-)
---
base-commit: 8967b2fe1b2aaae3c29e207d85fab901d580a2dc
change-id: 20260906-nohmp-next-771b029cc0f2

Best regards,
--  
Marc-André Lureau <marcandre.lureau@redhat.com>

[PATCH v2 1/9] tests/launchupdate-test: correct g_auto usage

[Marc-André Lureau]

g_free() isn't the appropriate way to release those types.

Fixes: 0b70c6ce7b31 ("Add functional and unit tests for the vm-launch-update device")
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/qtest/launchupdate-test.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/launchupdate-test.c b/tests/qtest/launchupdate-test.c
index 225c843df5b9..ed007cdce29f 100644
--- a/tests/qtest/launchupdate-test.c
+++ b/tests/qtest/launchupdate-test.c
@@ -192,8 +192,8 @@ static guint32 match_string(char *serial_f, const char *exp_out)
 {
     GError *error = NULL;
     g_autofree gchar *f_contents = NULL;
-    g_autofree GRegex *regex = NULL;
-    g_autofree GMatchInfo *match_info = NULL;
+    g_autoptr(GRegex) regex = NULL;
+    g_autoptr(GMatchInfo) match_info = NULL;
     gsize len;
     guint32 count = 0;
     gboolean ret;
@@ -214,7 +214,6 @@ static guint32 match_string(char *serial_f, const char *exp_out)
         g_match_info_next(match_info, &error);
         count++;
     }
-    g_regex_unref(regex);
     return count;
 }
 

-- 
2.56.0.116.g6de20f6092dc

[PATCH v2 2/9] tests/launchupdate-test: correctly release qs

[Marc-André Lureau]

QOSState must be released with qtest_shutdown().

Fixes: 0b70c6ce7b31 ("Add functional and unit tests for the vm-launch-update device")
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/qtest/launchupdate-test.c | 13 ++++---------
 tests/qtest/libqos/libqos.h     |  2 ++
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/launchupdate-test.c b/tests/qtest/launchupdate-test.c
index ed007cdce29f..ea35da81feb9 100644
--- a/tests/qtest/launchupdate-test.c
+++ b/tests/qtest/launchupdate-test.c
@@ -51,7 +51,7 @@ static void test_vm_launch_update_capability(void)
 static void test_vm_launch_update_disable(void)
 {
     QFWCFG *fw_cfg;
-    QOSState *qs;
+    g_autoptr(QOSState) qs = NULL;
     VMLaunchUpdate launch_update;
     uint64_t control;
     size_t filesize;
@@ -95,13 +95,12 @@ static void test_vm_launch_update_disable(void)
     g_assert_cmpint(VM_LAUNCHUPDATE_CTL_DISABLE & control, ==, 1);
 
     pc_fw_cfg_uninit(fw_cfg);
-    qtest_shutdown(qs);
 }
 
 static void check_error(void)
 {
     QFWCFG *fw_cfg;
-    QOSState *qs;
+    g_autoptr(QOSState) qs = NULL;
     VMLaunchUpdate launch_update;
     uint16_t status;
     size_t filesize;
@@ -326,7 +325,7 @@ static void test_load_igvm(void)
     size_t igvm_sz;
     size_t filesize;
     QFWCFG *fw_cfg;
-    QOSState *qs;
+    g_autoptr(QOSState) qs = NULL;
     VMLaunchUpdate launch_update;
 
     if (!trace) {
@@ -456,8 +455,6 @@ static void test_load_igvm(void)
     close(ser_fd);
     guest_free(&qs->alloc, gaddr);
     pc_fw_cfg_uninit(fw_cfg);
-    /* qtest_quit() kils QEMU, first by sending SIGTERM, then SIGKILL */
-    qtest_quit(qs->qts);
 }
 
 static void test_set_ctrl_once_and_reset_to_host_igvm(void)
@@ -475,7 +472,7 @@ static void test_set_ctrl_once_and_reset_to_host_igvm(void)
     size_t igvm_sz;
     size_t filesize;
     QFWCFG *fw_cfg;
-    QOSState *qs;
+    g_autoptr(QOSState) qs = NULL;
     VMLaunchUpdate launch_update;
 
     if (!qtest_has_machine("q35")) {
@@ -585,8 +582,6 @@ static void test_set_ctrl_once_and_reset_to_host_igvm(void)
     close(ser_fd);
     guest_free(&qs->alloc, gaddr);
     pc_fw_cfg_uninit(fw_cfg);
-    /* qtest_quit() kils QEMU, first by sending SIGTERM, then SIGKILL */
-    qtest_quit(qs->qts);
 }
 
 int main(int argc, char **argv)
diff --git a/tests/qtest/libqos/libqos.h b/tests/qtest/libqos/libqos.h
index c04950e2b19d..b2d02825a9af 100644
--- a/tests/qtest/libqos/libqos.h
+++ b/tests/qtest/libqos/libqos.h
@@ -42,4 +42,6 @@ static inline void qfree(QOSState *q, uint64_t addr)
     guest_free(&q->alloc, addr);
 }
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QOSState, qtest_shutdown)
+
 #endif

-- 
2.56.0.116.g6de20f6092dc

[PATCH v2 3/9] tests/launchupdate-test: fix fw_cfg leak

[Marc-André Lureau]

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/qtest/launchupdate-test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qtest/launchupdate-test.c b/tests/qtest/launchupdate-test.c
index ea35da81feb9..beec902caa6a 100644
--- a/tests/qtest/launchupdate-test.c
+++ b/tests/qtest/launchupdate-test.c
@@ -148,6 +148,8 @@ static void check_error(void)
     status = le64_to_cpu(launch_update.status);
     /* should fail with LOAD_FAIL since it was not IGVM format */
     g_assert_cmpint(status, ==, VM_LAUNCHUPDATE_LOAD_FAIL);
+
+    pc_fw_cfg_uninit(fw_cfg);
 }
 
 static int64_t get_image_size(const char *filename)

-- 
2.56.0.116.g6de20f6092dc

[PATCH v2 4/9] igvm: release found memory region on success

[Marc-André Lureau]

Not only in error paths must the mrs.mr be released.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 backends/igvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/backends/igvm.c b/backends/igvm.c
index 58f3097e8518..7aa76441d0d0 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -268,9 +268,9 @@ static void *qigvm_prepare_memory(QIgvm *ctx, uint64_t addr, uint64_t size,
             return NULL;
         }
 
+        memory_region_unref(mrs.mr);
         gpa_region_size = int128_make64(size);
         if (int128_lt(mrs.size, gpa_region_size)) {
-            memory_region_unref(mrs.mr);
             error_setg(
                 errp,
                 "Processing of IGVM file failed: Could not prepare memory "

-- 
2.56.0.116.g6de20f6092dc

[PATCH v2 5/9] igvm: make IgvmMemoryRegion a QOM

[Marc-André Lureau]

Make IgvmMemoryRegion a QOM object, embedding MemoryRegion, and use it
as the region owner. The IGVM list and memory subsystem can now hold
references independently, and QOM frees after the last reference is
dropped, fixing leaks reported by ASAN while running tests.

- object_unparent() is no longer needed, children are automatically unref
- switch to memory_region_init_ram_flags_nomigrate() as other helpers
  expect a DEVICE

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 backends/igvm.c                | 97 +++++++++++++++++++++++++-----------------
 include/system/igvm-internal.h |  9 ++--
 2 files changed, 64 insertions(+), 42 deletions(-)

diff --git a/backends/igvm.c b/backends/igvm.c
index 7aa76441d0d0..e659fea25517 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -35,6 +35,12 @@
 #define IGVM_VHT_OPTIONAL_BIT (1U << 31)
 #endif
 
+struct IgvmMemoryRegion {
+    Object parent_obj;
+    MemoryRegion mr;
+    QTAILQ_ENTRY(IgvmMemoryRegion) next;
+};
+
 /*
  * Bit 31 of the variable header type indicates that the header is
  * optional and can be safely ignored by a loader that does not
@@ -253,10 +259,13 @@ static void *qigvm_prepare_memory(QIgvm *ctx, uint64_t addr, uint64_t size,
                                   int region_identifier, Error **errp)
 {
     ERRP_GUARD();
-    IgvmMemoryRegion *imr = NULL;
+    g_autoptr(IgvmMemoryRegion) imr = NULL;
     Int128 gpa_region_size;
+    g_autofree char *region_name = NULL;
+    uint32_t flags = 0;
     MemoryRegionSection mrs =
         memory_region_find(get_system_memory(), addr, size);
+
     if (mrs.mr) {
         if (!memory_region_is_ram(mrs.mr)) {
             memory_region_unref(mrs.mr);
@@ -279,36 +288,29 @@ static void *qigvm_prepare_memory(QIgvm *ctx, uint64_t addr, uint64_t size,
             return NULL;
         }
         return qemu_map_ram_ptr(mrs.mr->ram_block, mrs.offset_within_region);
-    } else {
-        /*
-         * The region_identifier is the is the index of the IGVM directive that
-         * contains the page with the lowest GPA in the region. This will
-         * generate a unique region name.
-         */
-        g_autofree char *region_name =
-            g_strdup_printf("igvm.%X", region_identifier);
-        imr = g_new0(IgvmMemoryRegion, 1);
-        imr->mr = g_new0(MemoryRegion, 1);
-        if (ctx->machine_state->cgs &&
-            ctx->machine_state->cgs->require_guest_memfd) {
-            if (!memory_region_init_ram_guest_memfd_private(
-                    imr->mr, NULL, region_name, size, errp)) {
-                g_free(imr->mr);
-                g_free(imr);
-                return NULL;
-            }
-        } else {
-            if (!memory_region_init_ram(imr->mr, NULL, region_name, size,
-                                        errp)) {
-                g_free(imr->mr);
-                g_free(imr);
-                return NULL;
-            }
-        }
-        memory_region_add_subregion(get_system_memory(), addr, imr->mr);
-        QTAILQ_INSERT_TAIL(&ctx->cfg->memory_regions, imr, next);
-        return memory_region_get_ram_ptr(imr->mr);
     }
+
+    /*
+     * The region_identifier is the is the index of the IGVM directive that
+     * contains the page with the lowest GPA in the region. This will
+     * generate a unique region name.
+     */
+    region_name = g_strdup_printf("igvm.%X", region_identifier);
+    imr = IGVM_MEMORY_REGION(object_new(TYPE_IGVM_MEMORY_REGION));
+    if (ctx->machine_state->cgs &&
+        ctx->machine_state->cgs->require_guest_memfd) {
+        flags = RAM_GUEST_MEMFD_PRIVATE;
+    }
+    if (!memory_region_init_ram_flags_nomigrate(&imr->mr, OBJECT(imr),
+                                                region_name, size,
+                                                flags, errp)) {
+        return NULL;
+    }
+    vmstate_register_ram_global(&imr->mr);
+    memory_region_add_subregion(get_system_memory(), addr, &imr->mr);
+    object_ref(imr); /* for the list */
+    QTAILQ_INSERT_TAIL(&ctx->cfg->memory_regions, imr, next);
+    return memory_region_get_ram_ptr(&imr->mr);
 }
 
 static int qigvm_type_to_cgs_type(IgvmPageDataType memory_type, bool unmeasured,
@@ -1161,14 +1163,33 @@ void qigvm_cleanup_memory(IgvmCfg *cfg)
 {
     IgvmMemoryRegion *imr, *tmp;
 
-    QTAILQ_FOREACH_SAFE(imr, &cfg->memory_regions, next, tmp)
-    {
-        trace_qigvm_cleanup_memory(imr->mr->name);
-        memory_region_del_subregion(get_system_memory(), imr->mr);
-        vmstate_unregister_ram(imr->mr, NULL);
+    QTAILQ_FOREACH_SAFE(imr, &cfg->memory_regions, next, tmp) {
+        trace_qigvm_cleanup_memory(imr->mr.name);
         QTAILQ_REMOVE(&cfg->memory_regions, imr, next);
-        /* this triggers MemoryRegion cleanup */
-        object_unparent(OBJECT(imr->mr));
-        g_free(imr);
+        object_unref(OBJECT(imr));
     }
 }
+
+static void qigvm_memory_region_finalize(Object *obj)
+{
+    IgvmMemoryRegion *imr = IGVM_MEMORY_REGION(obj);
+
+    if (imr->mr.container) {
+        memory_region_del_subregion(imr->mr.container, &imr->mr);
+    }
+    vmstate_unregister_ram(&imr->mr, NULL);
+}
+
+static const TypeInfo qigvm_memory_region_info = {
+    .name = TYPE_IGVM_MEMORY_REGION,
+    .parent = TYPE_OBJECT,
+    .instance_size = sizeof(IgvmMemoryRegion),
+    .instance_finalize = qigvm_memory_region_finalize,
+};
+
+static void qigvm_register_types(void)
+{
+    type_register_static(&qigvm_memory_region_info);
+}
+
+type_init(qigvm_register_types);
diff --git a/include/system/igvm-internal.h b/include/system/igvm-internal.h
index b3b64cb58444..c2ceb57ffa65 100644
--- a/include/system/igvm-internal.h
+++ b/include/system/igvm-internal.h
@@ -18,10 +18,11 @@
 #include "system/confidential-guest-support.h"
 #include <igvm/igvm.h>
 
-typedef struct IgvmMemoryRegion {
-    QTAILQ_ENTRY(IgvmMemoryRegion) next;
-    MemoryRegion *mr;
-} IgvmMemoryRegion;
+typedef struct IgvmMemoryRegion IgvmMemoryRegion;
+
+#define TYPE_IGVM_MEMORY_REGION "igvm-memory-region"
+
+OBJECT_DECLARE_SIMPLE_TYPE(IgvmMemoryRegion, IGVM_MEMORY_REGION)
 
 struct IgvmCfg {
     Object parent_obj;

-- 
2.56.0.116.g6de20f6092dc

[PATCH v2 6/9] hw/s390x/ipl: fix short SCSI loadparm buffer over-read

[Marc-André Lureau]

s390_build_iplb() initially points lp at the eight-byte CCW loadparm
array. For SCSI devices, it can replace lp with a variable-length string
returned by object_property_get_str().

The following memcmp() still reads eight bytes. ASan reports
buffer-overflow on value "3", when running test_scsi_loadparm
func-s390x-boot_4k test.

Check the fixed-size CCW loadparm for the 8-zero before the SCSI
property can replace lp.

Fixes: 429442e52d94 ("hw: Add "loadparm" property to scsi disk devices for booting on s390x")
Reviewed-by: Jared Rossi <jrossi@linux.ibm.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/s390x/ipl.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 1add11ad7b2c..fe107af84de0 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -570,6 +570,11 @@ static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
     if (ccw_dev) {
         lp = ccw_dev->loadparm;
 
+        /* If the device loadparm is empty use the global machine loadparm */
+        if (memcmp(lp, NO_LOADPARM, 8) == 0) {
+            lp = S390_CCW_MACHINE(qdev_get_machine())->loadparm;
+        }
+
         switch (devtype) {
         case CCW_DEVTYPE_SCSI:
             sd = SCSI_DEVICE(dev_st);
@@ -605,11 +610,6 @@ static bool s390_build_iplb(DeviceState *dev_st, IplParameterBlock *iplb)
             break;
         }
 
-        /* If the device loadparm is empty use the global machine loadparm */
-        if (memcmp(lp, NO_LOADPARM, 8) == 0) {
-            lp = S390_CCW_MACHINE(qdev_get_machine())->loadparm;
-        }
-
         s390_ipl_convert_loadparm((char *)lp, iplb->loadparm);
         iplb->flags |= DIAG308_FLAGS_LP_VALID;
 

-- 
2.56.0.116.g6de20f6092dc

[PATCH v2 7/9] glib-compat: fix g_autofd fallback warnings

[Marc-André Lureau]

The g_autofd fallback is warning because it uses symbols from >=2.66

Fixes: 1599015a7a1a ("glib-compat: add fallback for g_clear_fd/g_autofd")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/glib-compat.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/glib-compat.h b/include/glib-compat.h
index 32ee2afdcdf5..dd6f062f5a43 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -148,15 +148,15 @@ static inline gboolean g_clear_fd_qemu(int *fd_ptr, GError **error)
 }
 #define g_clear_fd(fd, err) g_clear_fd_qemu(fd, err)
 
-#if !GLIB_CHECK_VERSION(2, 76, 0)
-static inline void _g_clear_fd_ignore_error(int *fd_ptr)
+static inline void _g_clear_fd_qemu_ignore_error(int *fd_ptr)
 {
     int errsv = errno;
     g_clear_fd(fd_ptr, NULL);
     errno = errsv;
 }
-#define g_autofd __attribute__((cleanup(_g_clear_fd_ignore_error)))
-#endif
+
+#undef g_autofd
+#define g_autofd __attribute__((cleanup(_g_clear_fd_qemu_ignore_error)))
 
 #pragma GCC diagnostic pop
 

-- 
2.56.0.116.g6de20f6092dc

Re: [PATCH v2 7/9] glib-compat: fix g_autofd fallback warnings

[Alex Bennée]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> The g_autofd fallback is warning because it uses symbols from >=2.66
>
> Fixes: 1599015a7a1a ("glib-compat: add fallback for g_clear_fd/g_autofd")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH v2 8/9] tests/exynos4210_uart_fifo-test: fix buffer leaks

[Marc-André Lureau]

Reported by ASAN.

Fixes: 6d459c75cfc4 ("tests/qtest: add test for exynos4210 uart fifo overflow bug")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/qtest/exynos4210_uart_fifo-test.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/exynos4210_uart_fifo-test.c b/tests/qtest/exynos4210_uart_fifo-test.c
index 54c09a6de723..c336ed22b028 100644
--- a/tests/qtest/exynos4210_uart_fifo-test.c
+++ b/tests/qtest/exynos4210_uart_fifo-test.c
@@ -52,8 +52,8 @@ static void enable_uart_fifo(QTestState *qts)
  */
 static void test_recv_large_str(void)
 {
-    int sock_fd;
-    char *sendbuf, *recvbuf;
+    g_autofd int sock_fd = -1;
+    g_autofree char *sendbuf = NULL, *recvbuf = NULL;
     QTestState *qts = qtest_init_with_serial("-M smdkc210", &sock_fd);
 
     enable_uart_fifo(qts);
@@ -71,8 +71,6 @@ static void test_recv_large_str(void)
     g_assert_true(TEST_BUFFER_LEN - 1 == strlen(recvbuf));
     g_assert_true(strcmp(sendbuf, recvbuf) == 0);
 
-    close(sock_fd);
-
     qtest_quit(qts);
 }
 

-- 
2.56.0.116.g6de20f6092dc

Re: [PATCH v2 8/9] tests/exynos4210_uart_fifo-test: fix buffer leaks

[Alex Bennée]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Reported by ASAN.
>
> Fixes: 6d459c75cfc4 ("tests/qtest: add test for exynos4210 uart fifo overflow bug")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/qtest/exynos4210_uart_fifo-test.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qtest/exynos4210_uart_fifo-test.c b/tests/qtest/exynos4210_uart_fifo-test.c
> index 54c09a6de723..c336ed22b028 100644
> --- a/tests/qtest/exynos4210_uart_fifo-test.c
> +++ b/tests/qtest/exynos4210_uart_fifo-test.c
> @@ -52,8 +52,8 @@ static void enable_uart_fifo(QTestState *qts)
>   */
>  static void test_recv_large_str(void)
>  {
> -    int sock_fd;
> -    char *sendbuf, *recvbuf;
> +    g_autofd int sock_fd = -1;
> +    g_autofree char *sendbuf = NULL, *recvbuf = NULL;
>      QTestState *qts = qtest_init_with_serial("-M smdkc210", &sock_fd);
>  
>      enable_uart_fifo(qts);
> @@ -71,8 +71,6 @@ static void test_recv_large_str(void)
>      g_assert_true(TEST_BUFFER_LEN - 1 == strlen(recvbuf));
>      g_assert_true(strcmp(sendbuf, recvbuf) == 0);
>  
> -    close(sock_fd);
> -

Did the close(sock_fd) fail or is this just adding another auto free
while cleaning up the buffer pointers? Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


>      qtest_quit(qts);
>  }

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v2 8/9] tests/exynos4210_uart_fifo-test: fix buffer leaks

[Marc-André Lureau]

Hi

On Wed, Oct 21, 2026 at 3:21 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
> > Reported by ASAN.
> >
> > Fixes: 6d459c75cfc4 ("tests/qtest: add test for exynos4210 uart fifo overflow bug")
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  tests/qtest/exynos4210_uart_fifo-test.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/qtest/exynos4210_uart_fifo-test.c b/tests/qtest/exynos4210_uart_fifo-test.c
> > index 54c09a6de723..c336ed22b028 100644
> > --- a/tests/qtest/exynos4210_uart_fifo-test.c
> > +++ b/tests/qtest/exynos4210_uart_fifo-test.c
> > @@ -52,8 +52,8 @@ static void enable_uart_fifo(QTestState *qts)
> >   */
> >  static void test_recv_large_str(void)
> >  {
> > -    int sock_fd;
> > -    char *sendbuf, *recvbuf;
> > +    g_autofd int sock_fd = -1;
> > +    g_autofree char *sendbuf = NULL, *recvbuf = NULL;
> >      QTestState *qts = qtest_init_with_serial("-M smdkc210", &sock_fd);
> >
> >      enable_uart_fifo(qts);
> > @@ -71,8 +71,6 @@ static void test_recv_large_str(void)
> >      g_assert_true(TEST_BUFFER_LEN - 1 == strlen(recvbuf));
> >      g_assert_true(strcmp(sendbuf, recvbuf) == 0);
> >
> > -    close(sock_fd);
> > -
>
> Did the close(sock_fd) fail or is this just adding another auto free
> while cleaning up the buffer pointers? Anyway:
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

No, it's just a cleanup while touching this code. We can remove it, or
adjust the commit message

Re: [PATCH v2 8/9] tests/exynos4210_uart_fifo-test: fix buffer leaks

[Alex Bennée]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Hi
>
> On Wed, Oct 21, 2026 at 3:21 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>
>> > Reported by ASAN.
>> >
>> > Fixes: 6d459c75cfc4 ("tests/qtest: add test for exynos4210 uart fifo overflow bug")
>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > ---
>> >  tests/qtest/exynos4210_uart_fifo-test.c | 6 ++----
>> >  1 file changed, 2 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/tests/qtest/exynos4210_uart_fifo-test.c b/tests/qtest/exynos4210_uart_fifo-test.c
>> > index 54c09a6de723..c336ed22b028 100644
>> > --- a/tests/qtest/exynos4210_uart_fifo-test.c
>> > +++ b/tests/qtest/exynos4210_uart_fifo-test.c
>> > @@ -52,8 +52,8 @@ static void enable_uart_fifo(QTestState *qts)
>> >   */
>> >  static void test_recv_large_str(void)
>> >  {
>> > -    int sock_fd;
>> > -    char *sendbuf, *recvbuf;
>> > +    g_autofd int sock_fd = -1;
>> > +    g_autofree char *sendbuf = NULL, *recvbuf = NULL;
>> >      QTestState *qts = qtest_init_with_serial("-M smdkc210", &sock_fd);
>> >
>> >      enable_uart_fifo(qts);
>> > @@ -71,8 +71,6 @@ static void test_recv_large_str(void)
>> >      g_assert_true(TEST_BUFFER_LEN - 1 == strlen(recvbuf));
>> >      g_assert_true(strcmp(sendbuf, recvbuf) == 0);
>> >
>> > -    close(sock_fd);
>> > -
>>
>> Did the close(sock_fd) fail or is this just adding another auto free
>> while cleaning up the buffer pointers? Anyway:
>>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
> No, it's just a cleanup while touching this code. We can remove it, or
> adjust the commit message

its fine to tweak the commit message...

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH v2 9/9] tests/xilinx-zynq-sdhci-test: fix GIOChannel leak

[Marc-André Lureau]

Reported by ASAN.

Fixes: 255c627c645b ("qtest: add xilinx-zynq SDHCI sdma test")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/qtest/xilinx-zynq-sdhci-test.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/xilinx-zynq-sdhci-test.c b/tests/qtest/xilinx-zynq-sdhci-test.c
index 7957358cf5d4..42e1b8f46e88 100644
--- a/tests/qtest/xilinx-zynq-sdhci-test.c
+++ b/tests/qtest/xilinx-zynq-sdhci-test.c
@@ -58,10 +58,10 @@ static QTestState *setup_sd_card(void)
 
 static void write_pattern_to_image(size_t offset, size_t len)
 {
-    gchar *buf = g_malloc(len);
+    g_autofree gchar *buf = g_malloc(len);
+    g_autoptr(GIOChannel) chan = NULL;
     GError *err = NULL;
     gsize bytes_written;
-    GIOChannel *chan;
 
     for (size_t i = 0; i < len; i++) {
         buf[i] = (i * 13 + 7) & 0xff;
@@ -78,8 +78,6 @@ static void write_pattern_to_image(size_t offset, size_t len)
     g_assert_cmpint(bytes_written, ==, len);
     g_io_channel_shutdown(chan, TRUE, &err);
     g_assert_no_error(err);
-
-    g_free(buf);
 }
 
 static void wait_for_block_count(QTestState *qts, uint16_t expected)

-- 
2.56.0.116.g6de20f6092dc

Re: [PATCH v2 9/9] tests/xilinx-zynq-sdhci-test: fix GIOChannel leak

[Philippe Mathieu-Daudé]

On 21/10/26 09:51, Marc-André Lureau wrote:
> Reported by ASAN.
> 
> Fixes: 255c627c645b ("qtest: add xilinx-zynq SDHCI sdma test")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   tests/qtest/xilinx-zynq-sdhci-test.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH v2 0/9] Various ASAN & tests fixes

[Gerd Hoffman]

On Wed, Oct 21, 2026 at 11:51:03AM +0400, Marc-André Lureau wrote:
> Hi,
> 
> A collection of leak-related fixes.

With this series applied I see a bunch of qom-test failures pop up.

take care,
  Gerd

Re: [PATCH v2 0/9] Various ASAN & tests fixes

[Gerd Hoffman]

On Wed, Oct 21, 2026 at 03:49:24PM +0200, Gerd Hoffman wrote:
> On Wed, Oct 21, 2026 at 11:51:03AM +0400, Marc-André Lureau wrote:
> > Hi,
> > 
> > A collection of leak-related fixes.
> 
> With this series applied I see a bunch of qom-test failures pop up.

Caused by "igvm: make IgvmMemoryRegion a QOM".

take care,
  Gerd