Re: [PATCH 4/8] perf machine: Check snprintf truncation in machines__findnew()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but missing NULL check for `machine` in event delivery causes NULL pointer dereference for non-SAMPLE events when guest machine creation fails.
--

--- Patch [4]: [PATCH 4/8] perf machine: Check snprintf truncation in machines__findnew() ---
commit 6d258a0499e2df813b5de954dd914c53ea5c78dc
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf machine: Check snprintf truncation in machines__findnew()

This commit adds a truncation check to the guestmount path built with
snprintf(). If truncation occurs, it bails out early by setting the machine
pointer to NULL and returning.

> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index baf855e596c26..05724277c2a97 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -333,7 +333,12 @@ struct machine *machines__findnew(struct machines *machines, pid_t pid)
>  	if ((pid != HOST_KERNEL_ID) &&
>  	    (pid != DEFAULT_GUEST_KERNEL_ID) &&
>  	    (symbol_conf.guestmount)) {
> -		snprintf(path, sizeof(path), "%s/%d", symbol_conf.guestmount, pid);
> +		if (snprintf(path, sizeof(path), "%s/%d",
> +			     symbol_conf.guestmount, pid) >= (int)sizeof(path)) {
> +			pr_err("Guest path too long for pid %d\n", pid);
> +			machine = NULL;
> +			goto out;

[Severity: High]
This isn't a bug introduced by this patch, but does returning NULL here
cause a NULL pointer dereference later in the event delivery pipeline?

When machines__findnew() returns NULL because of the truncated guestmount
path, the NULL propagates to machines__deliver_event(). For non-SAMPLE
events, it appears machine is passed to tool callbacks without validation:

tools/perf/util/session.c:machines__deliver_event() {
    ...
    return tool->mmap2(tool, event, sample, machine);
}

This flows into the callback and dereferences the NULL pointer:

tools/perf/util/machine.c:machine__process_mmap2_event() {
    ...
    ret = machine__process_kernel_mmap_event(machine, &xm, &dso_id.build_id);
}

tools/perf/util/machine.c:machine__process_kernel_mmap_event() {
    ...
    const char *mmap_name = machine->mmap_name;
}

Can we safely check for a NULL machine in machines__deliver_event() for
these non-SAMPLE events to prevent a segmentation fault?

> +		}
>  		if (access(path, R_OK)) {
>  			static struct strlist *seen;

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