Re: [PATCH 7/8] perf machine: Free scandir entries in guest kernel map creation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Crash during cleanup if `maps__insert()` fails when creating kernel maps.
--

--- Patch [7]: [PATCH 7/8] perf machine: Free scandir entries in guest kernel map creation ---
commit 39417761e2f7dcdcc875be7b6d926b0a7b9bade6
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf machine: Free scandir entries in guest kernel map creation

machines__create_guest_kernel_maps() calls scandir() which allocates
both the namelist array and each individual dirent entry.  The code
frees the namelist array but not the individual entries, leaking memory
proportional to the number of directories under guestmount.

> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 1700130adedb3..48c4b963e8097 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c

[ ... ]

> @@ -1265,6 +1266,7 @@ int machines__create_guest_kernel_maps(struct machines *machines)
>  			    (errno == ERANGE)) {
>  				pr_debug("invalid directory (%s). Skipping.\n",
>  					 namelist[i]->d_name);
> +				free(namelist[i]);
>  				continue;
>  			}
>  			snprintf(path, sizeof(path), "%s/%s/proc/kallsyms",
> @@ -1272,9 +1274,11 @@ int machines__create_guest_kernel_maps(struct machines *machines)
>  				 namelist[i]->d_name);
>  			if (access(path, R_OK)) {
>  				pr_debug("Can't access file %s\n", path);
> +				free(namelist[i]);
>  				continue;
>  			}
>  			machines__create_kernel_maps(machines, pid);

[Severity: High]
This is a pre-existing issue, but since we are executing the guest kernel
map creation path here, could this lead to an out-of-bounds memory access if
map insertion fails?

If machines__create_kernel_maps() ends up calling
__machine__create_kernel_maps() and maps__insert() fails:

tools/perf/util/machine.c:__machine__create_kernel_maps() {
    ...
    machine->vmlinux_map = map__new2(0, kernel);
    if (machine->vmlinux_map == NULL)
        return -ENOMEM;

    map__set_mapping_type(machine->vmlinux_map, MAPPING_TYPE__IDENTITY);
    return maps__insert(machine__kernel_maps(machine), machine->vmlinux_map);
}

A memory allocation failure in maps__insert() will return an error code, but
leave the uninserted map attached to machine->vmlinux_map. Later, during
teardown in machine__destroy_kernel_maps(), this dangling map is passed to
maps__remove().

Since it was never inserted, maps__by_address_index() in __maps__remove()
returns -1 (UINT_MAX):

tools/perf/util/maps.c:__maps__remove() {
    ...
    address_idx = maps__by_address_index(maps, map);
    map__put(maps_by_address[address_idx]);
    memmove(&maps_by_address[address_idx],
            &maps_by_address[address_idx + 1],
    ...
}

Does this use the UINT_MAX index directly to access and memmove the
maps_by_address array, causing an out-of-bounds memory access and potential
memory corruption during cleanup?

> +			free(namelist[i]);
>  		}
>  		free(namelist);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726234014.63111-1-acme@kernel.org?part=7