Re: [PATCH 1/9] perf machine: Propagate machine__init() error to callers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The return value of the newly fallible `machine__init()` function is ignored in tests, leading to potential null pointer dereferences on initialization failure.
--

commit 75ef0ba718fac4d2482673c59e4b0fde21e0bfe2
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf machine: Propagate machine__init() error to callers

machine__init() always returns 0 even when memory allocation fails,
because commit 81f981d7ec43ed93 introduced an error cleanup path but
left the final 'return 0' instead of 'return err'.

Fix by returning err, check the return value in __machine__new_host()
which was ignoring it, and change machines__init() from void to int so
it too can propagate the error.

> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 31715366e29ff..9329d319bd033 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
[ ... ]
> @@ -124,11 +123,11 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
>  
>  out:
>  	if (err) {
> -		zfree(&machine->kmaps);
> +		maps__zput(machine->kmaps);
>  		zfree(&machine->root_dir);
>  		zfree(&machine->mmap_name);
>  	}
> -	return 0;
> +	return err;
>  }

[Severity: Medium]
Since this patch changes machine__init() to return its internal error instead
of returning 0 on failure, do we also need to update the direct callers in the
tests to check this return value?

For example, test__kallsyms_split() ignores the return value:

tools/perf/tests/kallsyms-split.c:test__kallsyms_split() {
    ...
	machine__init(&m, root_dir, HOST_KERNEL_ID);
	if (machine__create_kernel_maps(&m) < 0) {
    ...
}

And test__vmlinux_matches_kallsyms() does as well:

tools/perf/tests/vmlinux-kallsyms.c:test__vmlinux_matches_kallsyms() {
    ...
	machine__init(&args.kallsyms, "", HOST_KERNEL_ID);
	machine__init(&vmlinux, "", HOST_KERNEL_ID);
    ...
}

If machine__init() fails, it cleans up and leaves machine.kmaps as NULL.

Could this lead to a NULL pointer dereference in functions like
machine__create_kernel_maps() that expect kmaps to be valid?

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