[BUG] in src/usr.sbin/inetd (?)

[tlaronde%polynum.com@localhost]

In /usr/src/usr.sbin/inetd/parse.c, a linked list of file_list
structures is included.

When adding a config include file, realpath(3) is called:

parse.c:include_configs()
1193 
	new_file.abs = realpath(CONFIG, NULL);

new_file.abs is not tested against NULL, and the structure is
inconditionnally added to the list. But before returning:

1201
	free(new_file.abs);

the allocated buffer is freed (may be not NULL) but the member is not
reset to NULL.

Then in check_no_reinclude(), the list is walked down and this test
is made:

1314
	char *abs_path = realpath(glob_path, NULL);

1324,1325
	for (cur = file_list_head; cur != NULL; cur = cur->next) {
		if (strcmp(cur->abs, abs_path) == 0) {

So:
	- if realpath(3) failed, strcmp(3) is called against NULL;
	- if realpath(3) succeeded, the comparison is made with a location
that has been freed!

It may be that the alloc engine doesn't reuse a freed region, so it may
work (if realpath(3) succeeded...), but it seems to me there is
something wrong... Am I reading the source correctly?

And on a minor style note:

In check_no_reinclude(), cur is assigned at declaration, but is in fact
redefined (to the same value) in the for loop:

1310,1325
static bool
check_no_reinclude(const char *glob_path)
{
	struct file_list *cur = file_list_head;
	char *abs_path = realpath(glob_path, NULL);

	if (abs_path == NULL) {
		ERR("Error checking real path for '%s': %s",
			glob_path, strerror(errno));
		return false;
	}

	DPRINTCONF("Absolute path '%s'", abs_path);

	for (cur = file_list_head; cur != NULL; cur = cur->next) {
		if (strcmp(cur->abs, abs_path) == 0) {

(abs_path is also assigned at declaration, but it is not redundant.)
-- 
        Thierry Laronde <tlaronde +AT+ polynum +dot+ com>
                     http://www.kergis.com/
                    http://kertex.kergis.com/
Key fingerprint = 0FF7 E906 FBAF FE95 FD89  250D 52B1 AE95 6006 F40C