Bug 246596 - Memory leak in ctld
Summary: Memory leak in ctld
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-20 09:40 UTC by Patryk
Modified: 2020-05-20 10:15 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Patryk 2020-05-20 09:40:11 UTC
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);
}
Comment 1 Patryk 2020-05-20 10:15:46 UTC
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.