From 675464b183f006fd805644075503f2d9bd647576 Mon Sep 17 00:00:00 2001 From: Daniel Veillard Date: Tue, 20 Sep 2011 11:51:50 +0800 Subject: [PATCH] Fix crash on events due to allocation errors remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError, remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics were using const string directly in rpc structure, before calling remoteDispatchDomainEventSend(). But that routine now frees up all the pointed allocated memory from the rpc structure and we end up with a double free. This now strdup() all the strings passed and provide mem_error goto labels to be used when an allocation error occurs. Note that the cleanup isn't completely finished because all relaying function also call make_nonnull_domain() which also allocate a string and never handle the error case. This patches doesn't try to address this as this is only error correctness a priori and touches far more functions in this module: * daemon/remote.c: fix string allocations and memory error handling for remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError, remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics --- daemon/remote.c | 103 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 81 insertions(+), 22 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 45244f8f1c..74e759a2f8 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -234,9 +234,13 @@ static int remoteRelayDomainEventIOError(virConnectPtr conn ATTRIBUTE_UNUSED, /* build return data */ memset(&data, 0, sizeof data); + data.srcPath = strdup(srcPath); + if (data.srcPath == NULL) + goto mem_error; + data.devAlias = strdup(devAlias); + if (data.devAlias == NULL) + goto mem_error; make_nonnull_domain(&data.dom, dom); - data.srcPath = (char*)srcPath; - data.devAlias = (char*)devAlias; data.action = action; remoteDispatchDomainEventSend(client, remoteProgram, @@ -244,6 +248,11 @@ static int remoteRelayDomainEventIOError(virConnectPtr conn ATTRIBUTE_UNUSED, (xdrproc_t)xdr_remote_domain_event_io_error_msg, &data); return 0; +mem_error: + virReportOOMError(); + virFree(data.srcPath); + virFree(data.devAlias); + return -1; } @@ -266,17 +275,31 @@ static int remoteRelayDomainEventIOErrorReason(virConnectPtr conn ATTRIBUTE_UNUS /* build return data */ memset(&data, 0, sizeof data); - make_nonnull_domain(&data.dom, dom); - data.srcPath = (char*)srcPath; - data.devAlias = (char*)devAlias; + data.srcPath = strdup(srcPath); + if (data.srcPath == NULL) + goto mem_error; + data.devAlias = strdup(devAlias); + if (data.devAlias == NULL) + goto mem_error; data.action = action; - data.reason = (char*)reason; + data.reason = strdup(reason); + if (data.reason == NULL) + goto mem_error; + + make_nonnull_domain(&data.dom, dom); remoteDispatchDomainEventSend(client, remoteProgram, REMOTE_PROC_DOMAIN_EVENT_IO_ERROR_REASON, (xdrproc_t)xdr_remote_domain_event_io_error_reason_msg, &data); return 0; + +mem_error: + virReportOOMError(); + virFree(data.srcPath); + virFree(data.devAlias); + virFree(data.reason); + return -1; } @@ -308,33 +331,62 @@ static int remoteRelayDomainEventGraphics(virConnectPtr conn ATTRIBUTE_UNUSED, /* build return data */ memset(&data, 0, sizeof data); - make_nonnull_domain(&data.dom, dom); data.phase = phase; - data.authScheme = (char*)authScheme; - data.local.family = local->family; - data.local.node = (char *)local->node; - data.local.service = (char *)local->service; - data.remote.family = remote->family; - data.remote.node = (char*)remote->node; - data.remote.service = (char*)remote->service; + data.authScheme = strdup(authScheme); + if (data.authScheme == NULL) + goto mem_error; + + data.local.node = strdup(local->node); + if (data.local.node == NULL) + goto mem_error; + data.local.service = strdup(local->service); + if (data.local.service == NULL) + goto mem_error; + + data.remote.node = strdup(remote->node); + if (data.remote.node == NULL) + goto mem_error; + data.remote.service = strdup(remote->service); + if (data.remote.service == NULL) + goto mem_error; data.subject.subject_len = subject->nidentity; - if (VIR_ALLOC_N(data.subject.subject_val, data.subject.subject_len) < 0) { - virReportOOMError(); - return -1; - } + if (VIR_ALLOC_N(data.subject.subject_val, data.subject.subject_len) < 0) + goto mem_error; + for (i = 0 ; i < data.subject.subject_len ; i++) { - data.subject.subject_val[i].type = (char*)subject->identities[i].type; - data.subject.subject_val[i].name = (char*)subject->identities[i].name; + data.subject.subject_val[i].type = strdup(subject->identities[i].type); + if (data.subject.subject_val[i].type == NULL) + goto mem_error; + data.subject.subject_val[i].name = strdup(subject->identities[i].name); + if (data.subject.subject_val[i].name == NULL) + goto mem_error; } + make_nonnull_domain(&data.dom, dom); remoteDispatchDomainEventSend(client, remoteProgram, REMOTE_PROC_DOMAIN_EVENT_GRAPHICS, (xdrproc_t)xdr_remote_domain_event_graphics_msg, &data); return 0; + +mem_error: + virReportOOMError(); + virFree(data.authScheme); + virFree(data.local.node); + virFree(data.local.service); + virFree(data.remote.node); + virFree(data.remote.service); + if (data.subject.subject_val != NULL) { + for (i = 0 ; i < data.subject.subject_len ; i++) { + virFree(data.subject.subject_val[i].type); + virFree(data.subject.subject_val[i].name); + } + virFree(data.subject.subject_val); + } + return -1; } static int remoteRelayDomainEventBlockJob(virConnectPtr conn ATTRIBUTE_UNUSED, @@ -355,16 +407,23 @@ static int remoteRelayDomainEventBlockJob(virConnectPtr conn ATTRIBUTE_UNUSED, /* build return data */ memset(&data, 0, sizeof data); - make_nonnull_domain(&data.dom, dom); - data.path = (char*)path; + data.path = strdup(path); + if (data.path == NULL) + goto mem_error; data.type = type; data.status = status; + make_nonnull_domain(&data.dom, dom); remoteDispatchDomainEventSend(client, remoteProgram, REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB, (xdrproc_t)xdr_remote_domain_event_block_job_msg, &data); return 0; + +mem_error: + virReportOOMError(); + virFree(data.path); + return -1; } -- 2.39.5