For a long time I had a mystery LSAN leak with:
Direct leak of 29 byte(s) in 1 object(s) allocated from: #0 0x7f7c696e7880 in malloc (/lib64/libasan.so.4+0xde880) #1 0x7f7c691afca4 in ber_memalloc_x (/lib64/liblber-2.4.so.2+0x7ca4) #2 0x60400000034f (<unknown module>)
I have finally solved the issue, which causes the basic tests to pass with ASAN + LSAN on rawhide now.
<img alt="0001-Ticket-49226-Memory-leak-in-ldap-agent-bin.patch" src="/389-ds-base/issue/raw/files/db4643f56c085a8b664f1247cdadf26bed2cff6d4b6156b28c442af06abd2c76-0001-Ticket-49226-Memory-leak-in-ldap-agent-bin.patch" />
23 passed in 192.05 seconds
Metadata Update from @firstyear: - Custom field reviewstatus adjusted to review - Custom field type adjusted to defect
Metadata Update from @mreynolds: - Custom field reviewstatus adjusted to ack (was: review)
64 + if ((strcmp(attr, "dn") == 0) && (strcmp(val, "cn=config") == 0)) { 65 char *dse_line = NULL; 66 - 67 - 68 + /* Free both outer values and attr */ 69 + free(attr); 70 + free(val); 71 + attr = NULL; 72 + val = NULL; 73 + 74 /* Look for port and rundir attributes */ 75 while ((dse_line = ldif_getline(&entryp)) != NULL) { 76 ldif_parse_line(dse_line, &attr, &val, &vlen); <== better check the return value as being done above? <== If failed, report an error and break? 77 @@ -447,6 +461,8 @@ load_config(char *conf_path) 78 } 79 got_rundir = 1; 80 } 81 + free(attr); 82 + free(val); <== you may want to set NULL to them? 83 84 /* Stop processing this entry if we found the 85 * port and rundir and snmp_index settings */ 86 @@ -458,6 +474,9 @@ load_config(char *conf_path) 87 * cn=config entry, so we can stop reading through 88 * the dse.ldif now. */ 89 break; 90 + } else { 91 + free(attr); 92 + free(val); <== you may want to set NULL to them? 93 } 94 } 95
i think you are right actually. It doesn't affect the fix, but it's "correct" to do this.
<img alt="0001-Ticket-49226-Memory-leak-in-ldap-agent-bin.patch" src="/389-ds-base/issue/raw/files/22d1ba5841179ed0f58838d1fc733669a1462b1c23398f0e4fb19618f918ea93-0001-Ticket-49226-Memory-leak-in-ldap-agent-bin.patch" />
How about adding an error checking for this line?
76 ldif_parse_line(dse_line, &attr, &val, &vlen);
If it fails, attr and val are going to be NULL...
That's also true, I didn't think of that. I'll add that too.
<img alt="0001-Ticket-49226-Memory-leak-in-ldap-agent-bin.patch" src="/389-ds-base/issue/raw/files/e142a1e553c2d8163d863405d1fde5edd82a2628cc855396fedd8d961f76152a-0001-Ticket-49226-Memory-leak-in-ldap-agent-bin.patch" />
William... Why you don't follow this way??? :)
415 if (ldif_parse_line(ldif_getline(&entryp), &attr, &val, &vlen)) { 416 printf("ldap-agent: error parsing ldif line from [%s]\n", serv_p->dse_ldif); 417 }
You know I could be a walking coverity... If you check attr OR val then continue, I'm pretty sure he complains about the case attr != NULL AND val == NULL and screams attr would be leaked. We know there's no such pair, but coverity won't be convinced... :)
75 while ((dse_line = ldif_getline(&entryp)) != NULL) { 76 ldif_parse_line(dse_line, &attr, &val, &vlen); 77 - if (strcmp(attr, "nsslapd-snmp-index") == 0) { 78 + if (attr == NULL || val == NULL) { 79 + /* can't parse these, try next line instead */ 80 + continue;
Ahhh! You are indeed correct. Clearly I should not be writing patches this morning if I miss something so obvious :)
<img alt="0001-Ticket-49226-Memory-leak-in-ldap-agent-bin.patch" src="/389-ds-base/issue/raw/files/6db7e5d2ca8176903e0d812e9c9f5439e69327f212e840b49310eb64f3fe2ac8-0001-Ticket-49226-Memory-leak-in-ldap-agent-bin.patch" />
Thanks, William! Ack.
commit 6ff44ae To ssh://git@pagure.io/389-ds-base.git f1e5692..6ff44ae master -> master
Metadata Update from @firstyear: - Issue close_status updated to: fixed - Issue status updated to: Closed (was: Open)
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/2285
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: fixed)
Login to comment on this ticket.