Re: [libvirt PATCH] remote: Fix memory leak in remoteDomainMigrateFinish3*
On Fri, Jan 27, 2023 at 02:37:29PM +0100, Jiri Denemark wrote: > Theoretically, when remoteDomainMigrateFinish3* is called without a > pointer for storing migration cookie and its length (i.e., either > cookieout == NULL or cookieoutlen == NULL), we would leak the freshly > created virDomain object referenced by rv. > > Signed-off-by: Jiri Denemark > --- Reviewed-by: Erik Skultety
[libvirt PATCH] remote: Fix memory leak in remoteDomainMigrateFinish3*
Theoretically, when remoteDomainMigrateFinish3* is called without a
pointer for storing migration cookie and its length (i.e., either
cookieout == NULL or cookieoutlen == NULL), we would leak the freshly
created virDomain object referenced by rv.
Signed-off-by: Jiri Denemark
---
src/remote/remote_driver.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 2f421fb5e0..9fc73f6d88 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -5917,8 +5917,6 @@ remoteDomainMigrateFinish3(virConnectPtr dconn,
(xdrproc_t) xdr_remote_domain_migrate_finish3_ret, (char *) )
== -1)
return NULL;
-rv = get_nonnull_domain(dconn, ret.dom);
-
if (ret.cookie_out.cookie_out_len > 0) {
if (!cookieout || !cookieoutlen) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -5930,6 +5928,8 @@ remoteDomainMigrateFinish3(virConnectPtr dconn,
ret.cookie_out.cookie_out_len = 0;
}
+rv = get_nonnull_domain(dconn, ret.dom);
+
xdr_free((xdrproc_t) _remote_domain_migrate_finish3_ret, (char *)
);
return rv;
@@ -6770,8 +6770,6 @@ remoteDomainMigrateFinish3Params(virConnectPtr dconn,
(char *) ) == -1)
goto cleanup;
-rv = get_nonnull_domain(dconn, ret.dom);
-
if (ret.cookie_out.cookie_out_len > 0) {
if (!cookieout || !cookieoutlen) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -6783,6 +6781,8 @@ remoteDomainMigrateFinish3Params(virConnectPtr dconn,
ret.cookie_out.cookie_out_len = 0;
}
+rv = get_nonnull_domain(dconn, ret.dom);
+
xdr_free((xdrproc_t) _remote_domain_migrate_finish3_params_ret,
(char *) );
--
2.39.1
[libvirt] [PATCH] remote: Fix memory leak in remoteConnectGetAllDomainStats
The remote call actually doesn't free the arguments array so we leak memory in case a domain list is specified. As the remote domain list array consists only of stolen pointers from the actual domain objects it's sufficient just to free the array. Valgrind message: ==1081452== 64 bytes in 1 blocks are definitely lost in loss record 632 of 726 ==1081452==at 0x4C296D0: calloc (vg_replace_malloc.c:618) ==1081452==by 0x4EA5CB4: virAllocN (viralloc.c:191) ==1081452==by 0x505D21E: remoteConnectGetAllDomainStats (remote_driver.c:7785) ==1081452==by 0x50081AA: virDomainListGetStats (libvirt-domain.c:11080) ==1081452==by 0x155249: cmdDomstats (virsh-domain-monitor.c:2147) ==1081452==by 0x12FB73: vshCommandRun (virsh.c:1935) ==1081452==by 0x133FEB: main (virsh.c:3719) --- src/remote/remote_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 65c04d9..d111e10 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7846,6 +7846,7 @@ remoteConnectGetAllDomainStats(virConnectPtr conn, VIR_FREE(elem); } virDomainStatsRecordListFree(tmpret); +VIR_FREE(args.doms.doms_val); xdr_free((xdrproc_t)xdr_remote_connect_get_all_domain_stats_ret, (char *) ret); -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] remote: Fix memory leak in remoteConnectGetAllDomainStats
On 05.11.2014 12:45, Peter Krempa wrote: The remote call actually doesn't free the arguments array so we leak memory in case a domain list is specified. As the remote domain list array consists only of stolen pointers from the actual domain objects it's sufficient just to free the array. Valgrind message: ==1081452== 64 bytes in 1 blocks are definitely lost in loss record 632 of 726 ==1081452==at 0x4C296D0: calloc (vg_replace_malloc.c:618) ==1081452==by 0x4EA5CB4: virAllocN (viralloc.c:191) ==1081452==by 0x505D21E: remoteConnectGetAllDomainStats (remote_driver.c:7785) ==1081452==by 0x50081AA: virDomainListGetStats (libvirt-domain.c:11080) ==1081452==by 0x155249: cmdDomstats (virsh-domain-monitor.c:2147) ==1081452==by 0x12FB73: vshCommandRun (virsh.c:1935) ==1081452==by 0x133FEB: main (virsh.c:3719) --- src/remote/remote_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 65c04d9..d111e10 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7846,6 +7846,7 @@ remoteConnectGetAllDomainStats(virConnectPtr conn, VIR_FREE(elem); } virDomainStatsRecordListFree(tmpret); +VIR_FREE(args.doms.doms_val); xdr_free((xdrproc_t)xdr_remote_connect_get_all_domain_stats_ret, (char *) ret); ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] remote: Fix memory leak on error path when deserializing bulk stats
The 'elem' variable along with the domain object would be leaked when
taking the error path.
Found by coverity.
---
src/remote/remote_driver.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index fda27f7..8bc4baa 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -7730,7 +7730,7 @@ remoteConnectGetAllDomainStats(virConnectPtr conn,
size_t i;
remote_connect_get_all_domain_stats_args args;
remote_connect_get_all_domain_stats_ret ret;
-
+virDomainStatsRecordPtr elem = NULL;
virDomainStatsRecordPtr *tmpret = NULL;
if (ndoms) {
@@ -7769,7 +7769,6 @@ remoteConnectGetAllDomainStats(virConnectPtr conn,
goto cleanup;
for (i = 0; i ret.retStats.retStats_len; i++) {
-virDomainStatsRecordPtr elem;
remote_domain_stats_record *rec = ret.retStats.retStats_val + i;
if (VIR_ALLOC(elem) 0)
@@ -7786,6 +7785,7 @@ remoteConnectGetAllDomainStats(virConnectPtr conn,
goto cleanup;
tmpret[i] = elem;
+elem = NULL;
}
*retStats = tmpret;
@@ -7793,6 +7793,10 @@ remoteConnectGetAllDomainStats(virConnectPtr conn,
rv = ret.retStats.retStats_len;
cleanup:
+if (elem) {
+virObjectUnref(elem-dom);
+VIR_FREE(elem);
+}
virDomainStatsRecordListFree(tmpret);
xdr_free((xdrproc_t)xdr_remote_connect_get_all_domain_stats_ret,
(char *) ret);
--
2.0.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] remote: Fix memory leak on error path when deserializing bulk stats
On Tue, Sep 02, 2014 at 03:18:39PM +0200, Peter Krempa wrote:
The 'elem' variable along with the domain object would be leaked when
taking the error path.
Found by coverity.
---
src/remote/remote_driver.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
ACK,
Martin
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index fda27f7..8bc4baa 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -7730,7 +7730,7 @@ remoteConnectGetAllDomainStats(virConnectPtr conn,
size_t i;
remote_connect_get_all_domain_stats_args args;
remote_connect_get_all_domain_stats_ret ret;
-
+virDomainStatsRecordPtr elem = NULL;
virDomainStatsRecordPtr *tmpret = NULL;
if (ndoms) {
@@ -7769,7 +7769,6 @@ remoteConnectGetAllDomainStats(virConnectPtr conn,
goto cleanup;
for (i = 0; i ret.retStats.retStats_len; i++) {
-virDomainStatsRecordPtr elem;
remote_domain_stats_record *rec = ret.retStats.retStats_val + i;
if (VIR_ALLOC(elem) 0)
@@ -7786,6 +7785,7 @@ remoteConnectGetAllDomainStats(virConnectPtr conn,
goto cleanup;
tmpret[i] = elem;
+elem = NULL;
}
*retStats = tmpret;
@@ -7793,6 +7793,10 @@ remoteConnectGetAllDomainStats(virConnectPtr conn,
rv = ret.retStats.retStats_len;
cleanup:
+if (elem) {
+virObjectUnref(elem-dom);
+VIR_FREE(elem);
+}
virDomainStatsRecordListFree(tmpret);
xdr_free((xdrproc_t)xdr_remote_connect_get_all_domain_stats_ret,
(char *) ret);
--
2.0.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] remote: Fix memory leak on error path when deserializing bulk stats
On 09/02/14 15:27, Martin Kletzander wrote: On Tue, Sep 02, 2014 at 03:18:39PM +0200, Peter Krempa wrote: The 'elem' variable along with the domain object would be leaked when taking the error path. Found by coverity. --- src/remote/remote_driver.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) ACK, Martin Pushed; Thanks. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] remote: Fix memory leak
On 07/10/2011 11:14 PM, a...@redhat.com wrote: From: Alex Jia a...@redhat.com Detected in valgrind run: ==9184== 1 bytes in 1 blocks are definitely lost in loss record 1 of 19 ==9184==at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==9184==by 0x3073715F78: xdr_array (xdr_array.c:97) ==9184==by 0x4CF97C9: xdr_remote_domain_get_security_label_ret (remote_protocol.c:1696) * src/remote/remote_driver.c: Avoid leak on remoteDomainGetSecurityLabel and remoteNodeGetSecurityModel. --- src/remote/remote_driver.c | 12 +--- 1 files changed, 9 insertions(+), 3 deletions(-) ACK and pushed. I added you to AUTHORS; let me know if I need to adjust anything for your preferred spellings. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] remote: fix memory leak
Detected in this valgrind run:
https://bugzilla.redhat.com/show_bug.cgi?id=690734
==13864== 10 bytes in 1 blocks are definitely lost in loss record 10 of 34
==13864==at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==13864==by 0x308587FD91: strdup (in /lib64/libc-2.12.so)
==13864==by 0x3BB68BA0A3: doRemoteOpen (remote_driver.c:451)
==13864==by 0x3BB68BCFCF: remoteOpen (remote_driver.c:)
==13864==by 0x3BB689A0FF: do_open (libvirt.c:1290)
==13864==by 0x3BB689ADD5: virConnectOpenAuth (libvirt.c:1545)
==13864==by 0x41F659: main (virsh.c:11613)
* src/remote/remote_driver.c (doRemoteClose): Move up.
(remoteOpenSecondaryDriver, remoteOpen): Avoid leak on failure.
---
src/remote/remote_driver.c | 120 +++-
1 files changed, 62 insertions(+), 58 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index b05bbcb..4c7df17 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -998,6 +998,64 @@ retry:
goto cleanup;
}
+static int
+doRemoteClose (virConnectPtr conn, struct private_data *priv)
+{
+if (priv-eventFlushTimer = 0) {
+/* Remove timeout */
+virEventRemoveTimeout(priv-eventFlushTimer);
+/* Remove handle for remote events */
+virEventRemoveHandle(priv-watch);
+priv-watch = -1;
+}
+
+if (call (conn, priv, 0, REMOTE_PROC_CLOSE,
+ (xdrproc_t) xdr_void, (char *) NULL,
+ (xdrproc_t) xdr_void, (char *) NULL) == -1)
+return -1;
+
+/* Close socket. */
+if (priv-uses_tls priv-session) {
+gnutls_bye (priv-session, GNUTLS_SHUT_RDWR);
+gnutls_deinit (priv-session);
+}
+#if HAVE_SASL
+if (priv-saslconn)
+sasl_dispose (priv-saslconn);
+#endif
+VIR_FORCE_CLOSE(priv-sock);
+VIR_FORCE_CLOSE(priv-errfd);
+
+#ifndef WIN32
+if (priv-pid 0) {
+pid_t reap;
+do {
+retry:
+reap = waitpid(priv-pid, NULL, 0);
+if (reap == -1 errno == EINTR)
+goto retry;
+} while (reap != -1 reap != priv-pid);
+}
+#endif
+VIR_FORCE_CLOSE(priv-wakeupReadFD);
+VIR_FORCE_CLOSE(priv-wakeupSendFD);
+
+
+/* Free hostname copy */
+VIR_FREE(priv-hostname);
+
+/* See comment for remoteType. */
+VIR_FREE(priv-type);
+
+/* Free callback list */
+virDomainEventCallbackListFree(priv-callbackList);
+
+/* Free queued events */
+virDomainEventQueueFree(priv-domainEvents);
+
+return 0;
+}
+
static struct private_data *
remoteAllocPrivateData(void)
{
@@ -1039,7 +1097,9 @@ remoteOpenSecondaryDriver(virConnectPtr conn,
ret = doRemoteOpen(conn, *priv, auth, rflags);
if (ret != VIR_DRV_OPEN_SUCCESS) {
+doRemoteClose(conn, *priv);
remoteDriverUnlock(*priv);
+virMutexDestroy((*priv)-lock);
VIR_FREE(*priv);
} else {
(*priv)-localUses = 1;
@@ -,7 +1171,9 @@ remoteOpen (virConnectPtr conn,
ret = doRemoteOpen(conn, priv, auth, rflags);
if (ret != VIR_DRV_OPEN_SUCCESS) {
conn-privateData = NULL;
+doRemoteClose(conn, priv);
remoteDriverUnlock(priv);
+virMutexDestroy(priv-lock);
VIR_FREE(priv);
} else {
conn-privateData = priv;
@@ -1538,64 +1600,6 @@ verify_certificate (virConnectPtr conn ATTRIBUTE_UNUSED,
static int
-doRemoteClose (virConnectPtr conn, struct private_data *priv)
-{
-if (priv-eventFlushTimer = 0) {
-/* Remove timeout */
-virEventRemoveTimeout(priv-eventFlushTimer);
-/* Remove handle for remote events */
-virEventRemoveHandle(priv-watch);
-priv-watch = -1;
-}
-
-if (call (conn, priv, 0, REMOTE_PROC_CLOSE,
- (xdrproc_t) xdr_void, (char *) NULL,
- (xdrproc_t) xdr_void, (char *) NULL) == -1)
-return -1;
-
-/* Close socket. */
-if (priv-uses_tls priv-session) {
-gnutls_bye (priv-session, GNUTLS_SHUT_RDWR);
-gnutls_deinit (priv-session);
-}
-#if HAVE_SASL
-if (priv-saslconn)
-sasl_dispose (priv-saslconn);
-#endif
-VIR_FORCE_CLOSE(priv-sock);
-VIR_FORCE_CLOSE(priv-errfd);
-
-#ifndef WIN32
-if (priv-pid 0) {
-pid_t reap;
-do {
-retry:
-reap = waitpid(priv-pid, NULL, 0);
-if (reap == -1 errno == EINTR)
-goto retry;
-} while (reap != -1 reap != priv-pid);
-}
-#endif
-VIR_FORCE_CLOSE(priv-wakeupReadFD);
-VIR_FORCE_CLOSE(priv-wakeupSendFD);
-
-
-/* Free hostname copy */
-VIR_FREE(priv-hostname);
-
-/* See comment for remoteType. */
-VIR_FREE(priv-type);
-
-/* Free callback list */
-virDomainEventCallbackListFree(priv-callbackList);
-
-/* Free queued events */
-virDomainEventQueueFree(priv-domainEvents);
-
-return 0;
-}
-
-static int
remoteClose (virConnectPtr conn)
{
int ret = 0;
--
1.7.4
--
libvir-list mailing
