Skip to content

Commit 978a518

Browse files
committed
Low: libcib: Avoid memory leak in processing CIB file commit transaction
process_transaction_requests() wasn't freeing output. Do that now. Also simplify the freeing of output. * process_request(): Make a copy of *output if it's not in the same doc as private->cib_xml. This lets us simplify file_perform_op_delegate(). * process_request(): No operation leaves result_cib set to the same value as *output unless it's also equal to private->cib_xml. (A query op can do this.) So drop a check in the done section. * file_perform_op_delegate(): Return only at the end of the function, within the done section. * file_perform_op_delegate(): Don't assign *output_data = NULL at the beginning. We'll do this in the done section if output is NULL. * file_perform_op_delegate(): Assume that output is not in the same document as private->cib_xml. We ensured this in process_request(). * file_perform_op_delegate(): Free output if and only if it has not been assigned to *output_data. And do the standard-to-legacy conversion at the end of file_perform_op_delegate(), in the done section. Signed-off-by: Reid Wahl <nrwahl@protonmail.com>
1 parent 8c38e6f commit 978a518

1 file changed

Lines changed: 29 additions & 30 deletions

File tree

lib/cib/cib_file.c

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,16 @@ process_request(cib_t *cib, xmlNode *request, xmlNode **output)
191191
set_file_flags(private, file_flag_dirty);
192192
}
193193

194+
if (*output == NULL) {
195+
goto done;
196+
}
197+
198+
if ((*output)->doc == private->cib_xml->doc) {
199+
*output = pcmk__xml_copy(NULL, *output);
200+
}
201+
194202
done:
195-
if ((result_cib != private->cib_xml) && (result_cib != *output)) {
203+
if (result_cib != private->cib_xml) {
196204
pcmk__xml_free(result_cib);
197205
}
198206
pcmk__xml_free(cib_diff);
@@ -226,6 +234,8 @@ process_transaction_requests(cib_t *cib, xmlNode *transaction)
226234

227235
int rc = process_request(cib, request, &output);
228236

237+
pcmk__xml_free(output);
238+
229239
if (rc != pcmk_rc_ok) {
230240
pcmk__err("Aborting transaction for CIB file client (%s) on file "
231241
"'%s' due to failed %s request: %s",
@@ -403,69 +413,58 @@ file_perform_op_delegate(cib_t *cib, const char *op, const char *host,
403413

404414
const cib__operation_t *operation = NULL;
405415

406-
pcmk__info("Handling %s operation for %s as %s",
407-
pcmk__s(op, "invalid"), pcmk__s(section, "entire CIB"),
416+
pcmk__info("Handling %s operation for %s as %s", pcmk__s(op, "invalid"),
417+
pcmk__s(section, "entire CIB"),
408418
pcmk__s(user_name, "default user"));
409419

410-
if (output_data != NULL) {
411-
*output_data = NULL;
412-
}
413-
414420
if (cib->state == cib_disconnected) {
415-
return -ENOTCONN;
421+
rc = ENOTCONN;
422+
goto done;
416423
}
417424

418425
rc = cib__get_operation(op, &operation);
419-
rc = pcmk_rc2legacy(rc);
420-
if (rc != pcmk_ok) {
426+
if (rc != pcmk_rc_ok) {
421427
// @COMPAT: At compatibility break, use rc directly
422-
return -EPROTONOSUPPORT;
428+
rc = EPROTONOSUPPORT;
429+
goto done;
423430
}
424431

425432
if (get_op_function(operation) == NULL) {
426433
// @COMPAT: At compatibility break, use EOPNOTSUPP
427434
pcmk__err("Operation %s is not supported by CIB file clients", op);
428-
return -EPROTONOSUPPORT;
435+
rc = EPROTONOSUPPORT;
436+
goto done;
429437
}
430438

431439
cib__set_call_options(call_options, "file operation", cib_no_mtime);
432440

433441
rc = cib__create_op(cib, op, host, section, data, call_options, user_name,
434442
NULL, &request);
435-
rc = pcmk_rc2legacy(rc);
436-
if (rc != pcmk_ok) {
437-
return rc;
443+
if (rc != pcmk_rc_ok) {
444+
goto done;
438445
}
439446

440447
pcmk__xe_set(request, PCMK__XA_ACL_TARGET, user_name);
441448
pcmk__xe_set(request, PCMK__XA_CIB_CLIENTID, private->id);
442449

443450
if (pcmk__is_set(call_options, cib_transaction)) {
444451
rc = cib__extend_transaction(cib, request);
445-
rc = pcmk_rc2legacy(rc);
446452
goto done;
447453
}
448454

449455
rc = process_request(cib, request, &output);
450-
rc = pcmk_rc2legacy(rc);
451-
452-
if ((output_data != NULL) && (output != NULL)) {
453-
if (output->doc == private->cib_xml->doc) {
454-
*output_data = pcmk__xml_copy(NULL, output);
455-
} else {
456-
*output_data = output;
457-
}
458-
}
459456

460457
done:
461-
if ((output != NULL)
462-
&& (output->doc != private->cib_xml->doc)
463-
&& ((output_data == NULL) || (output != *output_data))) {
458+
pcmk__xml_free(request);
464459

460+
if (output_data != NULL) {
461+
*output_data = output;
462+
463+
} else {
465464
pcmk__xml_free(output);
466465
}
467-
pcmk__xml_free(request);
468-
return rc;
466+
467+
return pcmk_rc2legacy(rc);
469468
}
470469

471470
/*!

0 commit comments

Comments
 (0)