Bug 246596
| Summary: | Memory leak in ctld | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Patryk <patrykkotlowski> |
| Component: | bin | Assignee: | Mark Johnston <markj> |
| Status: | Closed FIXED | ||
| Severity: | Affects Many People | CC: | markj |
| Priority: | --- | ||
| Version: | CURRENT | ||
| Hardware: | Any | ||
| OS: | Any | ||
Also I found another possible memory leak. In ctld.c in conf_apply fucntion there is:
/*
* Second, remove any LUNs present in the old configuration
* and missing in the new one.
*/
TAILQ_FOREACH_SAFE(oldlun, &oldconf->conf_luns, l_next, tmplun) {
newlun = lun_find(newconf, oldlun->l_name);
if (newlun == NULL) {
log_debugx("lun \"%s\", CTL lun %d "
"not found in new configuration; "
"removing", oldlun->l_name, oldlun->l_ctl_lun);
error = kernel_lun_remove(oldlun);
if (error != 0) {
log_warnx("failed to remove lun \"%s\", "
"CTL lun %d",
oldlun->l_name, oldlun->l_ctl_lun);
cumulated_error++;
}
continue;
}
Why here lun_delete function is not called before "continue;"?
I next part of function where luns with changed size are deleted there is lun_delete() call:
if (changed) {
error = kernel_lun_remove(oldlun);
if (error != 0) {
log_warnx("failed to remove lun \"%s\", "
"CTL lun %d",
oldlun->l_name, oldlun->l_ctl_lun);
cumulated_error++;
}
lun_delete(oldlun);
continue;
}
I did't tested in but I think this is also memory leak.
(In reply to Patryk from comment #1) I suspect there is a similar leak when kernel_port_remove() fails. A commit references this bug: Author: markj Date: Sat May 30 19:11:41 UTC 2020 New revision: 361654 URL: https://svnweb.freebsd.org/changeset/base/361654 Log: ctld: Fix a memory leak in uclparse_conf(). PR: 246596 Submitted by: Patryk <patrykkotlowski@gmail.com> MFC after: 1 week Changes: head/usr.sbin/ctld/uclparse.c I submitted https://reviews.freebsd.org/D25074 for the second bug. (In reply to Mark Johnston from comment #4) mav@ points out that conf_delete() will handle this, so I think only the parser code change is required. (In reply to Mark Johnston from comment #5) Sounds great to me. Thank you :) Patryk A commit references this bug: Author: markj Date: Wed Jun 3 14:54:55 UTC 2020 New revision: 361753 URL: https://svnweb.freebsd.org/changeset/base/361753 Log: MFC r361654: ctld: Fix a memory leak in uclparse_conf(). PR: 246596 Changes: _U stable/12/ stable/12/usr.sbin/ctld/uclparse.c Thanks for the report. |
In ucl_parse.c file there is memory leak: int uclparse_conf(struct conf *newconf, const char *path) { struct ucl_parser *parser; int error; conf = newconf; parser = ucl_parser_new(0); if (!ucl_parser_add_file(parser, path)) { log_warn("unable to parse configuration file %s: %s", path, ucl_parser_get_error(parser)); return (1); } error = uclparse_toplevel(ucl_parser_get_object(parser)); return (error); } ucl_parser pointer is never freed. My fix proposal is following (I tested it in my production environment): int uclparse_conf(struct conf *newconf, const char *path) { struct ucl_parser *parser; int error; conf = newconf; parser = ucl_parser_new(0); if (!ucl_parser_add_file(parser, path)) { log_warn("unable to parse configuration file %s: %s", path, ucl_parser_get_error(parser)); return (1); } ucl_object_t *p_top = ucl_parser_get_object(parser); error = uclparse_toplevel(p_top); ucl_object_unref(p_top); ucl_parser_free(parser); return (error); }