[PATCH] virnetdevtap: Fix memory leak in virNetDevTapReattachBridge
From: QiangWei Zhang <zhang.qiangwei@zte.com.cn> Variable 'master' needs to be free because it will be reassigned in virNetDevOpenvswitchInterfaceGetMaster(). The leaked stack: Direct leak of 11 byte(s) in 1 object(s) allocated from: #0 0x7f7dad8ba6df in __interceptor_malloc (/lib64/libasan.so.8+0xba6df) #1 0x7f7dad715728 in g_malloc (/lib64/libglib-2.0.so.0+0x60728) #2 0x7f7dad72d8b2 in g_strdup (/lib64/libglib-2.0.so.0+0x788b2) #3 0x7f7dacb63088 in g_strdup_inline /usr/include/glib-2.0/glib/gstrfuncs.h:321 #4 0x7f7dacb63088 in virNetDevGetName ../src/util/virnetdev.c:823 #5 0x7f7dacb63886 in virNetDevGetMaster ../src/util/virnetdev.c:909 #6 0x7f7dacb90288 in virNetDevTapReattachBridge ../src/util/virnetdevtap.c:527 #7 0x7f7dacd5cd67 in virDomainNetNotifyActualDevice ../src/conf/domain_conf.c:30505 #8 0x7f7da3a10bc3 in qemuProcessNotifyNets ../src/qemu/qemu_process.c:3290 #9 0x7f7da3a375c6 in qemuProcessReconnect ../src/qemu/qemu_process.c:9211 #10 0x7f7dacc0cc53 in virThreadHelper ../src/util/virthread.c:256 #11 0x7f7dac2875d4 in start_thread (/lib64/libc.so.6+0x875d4) #12 0x7f7dac3091bb in __GI___clone3 (/lib64/libc.so.6+0x1091bb) Signed-off-by: QiangWei Zhang <zhang.qiangwei@zte.com.cn> --- src/util/virnetdevtap.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 1dc77f0f5c..860a8e2dd5 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -541,6 +541,9 @@ virNetDevTapReattachBridge(const char *tapname, /* IFLA_MASTER for a tap on an OVS switch is always "ovs-system" */ if (STREQ_NULLABLE(master, "ovs-system")) { useOVS = true; + + /* master needs to be released here because it will be reassigned */ + VIR_FREE(master); if (virNetDevOpenvswitchInterfaceGetMaster(tapname, &master) < 0) return -1; } -- 2.27.0
On Tue, May 06, 2025 at 10:47:34 +0800, liu.song13@zte.com.cn wrote:
From: QiangWei Zhang <zhang.qiangwei@zte.com.cn>
Variable 'master' needs to be free because it will be reassigned in virNetDevOpenvswitchInterfaceGetMaster().
The leaked stack: Direct leak of 11 byte(s) in 1 object(s) allocated from: #0 0x7f7dad8ba6df in __interceptor_malloc (/lib64/libasan.so.8+0xba6df) #1 0x7f7dad715728 in g_malloc (/lib64/libglib-2.0.so.0+0x60728) #2 0x7f7dad72d8b2 in g_strdup (/lib64/libglib-2.0.so.0+0x788b2) #3 0x7f7dacb63088 in g_strdup_inline /usr/include/glib-2.0/glib/gstrfuncs.h:321 #4 0x7f7dacb63088 in virNetDevGetName ../src/util/virnetdev.c:823 #5 0x7f7dacb63886 in virNetDevGetMaster ../src/util/virnetdev.c:909 #6 0x7f7dacb90288 in virNetDevTapReattachBridge ../src/util/virnetdevtap.c:527 #7 0x7f7dacd5cd67 in virDomainNetNotifyActualDevice ../src/conf/domain_conf.c:30505 #8 0x7f7da3a10bc3 in qemuProcessNotifyNets ../src/qemu/qemu_process.c:3290 #9 0x7f7da3a375c6 in qemuProcessReconnect ../src/qemu/qemu_process.c:9211 #10 0x7f7dacc0cc53 in virThreadHelper ../src/util/virthread.c:256 #11 0x7f7dac2875d4 in start_thread (/lib64/libc.so.6+0x875d4) #12 0x7f7dac3091bb in __GI___clone3 (/lib64/libc.so.6+0x1091bb)
Signed-off-by: QiangWei Zhang <zhang.qiangwei@zte.com.cn> --- src/util/virnetdevtap.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 1dc77f0f5c..860a8e2dd5 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -541,6 +541,9 @@ virNetDevTapReattachBridge(const char *tapname, /* IFLA_MASTER for a tap on an OVS switch is always "ovs-system" */ if (STREQ_NULLABLE(master, "ovs-system")) { useOVS = true; + + /* master needs to be released here because it will be reassigned */ + VIR_FREE(master); if (virNetDevOpenvswitchInterfaceGetMaster(tapname, &master) < 0) return -1; }
Thanks for the patch. It seems to be technically correct, but it won't apply as it is missing any indentation. Could you please resend the email in a way that preserves leading spaces? And while at it, I don't think the comment provides any additional value. You can also add the following line to the commit message: Fixes: de938b92c9d3a47647164aa643c20d2fc96cd2bc Jirka
On Tue, May 06, 2025 at 10:47:34 +0800, liu.song13@zte.com.cn wrote:
From: QiangWei Zhang <zhang.qiangwei@zte.com.cn>
Variable 'master' needs to be free because it will be reassigned in virNetDevOpenvswitchInterfaceGetMaster().
The leaked stack: Direct leak of 11 byte(s) in 1 object(s) allocated from: #0 0x7f7dad8ba6df in __interceptor_malloc (/lib64/libasan.so.8+0xba6df) #1 0x7f7dad715728 in g_malloc (/lib64/libglib-2.0.so.0+0x60728) #2 0x7f7dad72d8b2 in g_strdup (/lib64/libglib-2.0.so.0+0x788b2) #3 0x7f7dacb63088 in g_strdup_inline /usr/include/glib-2.0/glib/gstrfuncs.h:321 #4 0x7f7dacb63088 in virNetDevGetName ../src/util/virnetdev.c:823 #5 0x7f7dacb63886 in virNetDevGetMaster ../src/util/virnetdev.c:909 #6 0x7f7dacb90288 in virNetDevTapReattachBridge ../src/util/virnetdevtap.c:527 #7 0x7f7dacd5cd67 in virDomainNetNotifyActualDevice ../src/conf/domain_conf.c:30505 #8 0x7f7da3a10bc3 in qemuProcessNotifyNets ../src/qemu/qemu_process.c:3290 #9 0x7f7da3a375c6 in qemuProcessReconnect ../src/qemu/qemu_process.c:9211 #10 0x7f7dacc0cc53 in virThreadHelper ../src/util/virthread.c:256 #11 0x7f7dac2875d4 in start_thread (/lib64/libc.so.6+0x875d4) #12 0x7f7dac3091bb in __GI___clone3 (/lib64/libc.so.6+0x1091bba
Looks like this was broken since virNetDevTapReattachBridge was introduced. Add: Fixes: de938b92c9d3a47647164aa643c20d2fc96cd2bc
Signed-off-by: QiangWei Zhang <zhang.qiangwei@zte.com.cn> --- src/util/virnetdevtap.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 1dc77f0f5c..860a8e2dd5 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -541,6 +541,9 @@ virNetDevTapReattachBridge(const char *tapname, /* IFLA_MASTER for a tap on an OVS switch is always "ovs-system" */ if (STREQ_NULLABLE(master, "ovs-system")) { useOVS = true; + + /* master needs to be released here because it will be reassigned */ + VIR_FREE(master); if (virNetDevOpenvswitchInterfaceGetMaster(tapname, &master) < 0) return -1; }
The patch (whitespace) got corrupted somehow. Generally we prefer if 'g_autofree' is not mixed with 'VIR_FREE' although here it's probably okay. You can use: Reviewed-by: Peter Krempa <pkrempa@redhat.com> when resending the fixed patch.
participants (3)
-
Jiri Denemark -
liu.song13＠zte.com.cn -
Peter Krempa