Re: [PATCH] tools/lib/api: Check nonfilterable in fdarray__filter()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] If `realloc` for `fda->priv` fails in `fdarray__grow()`, a dangling pointer is left in `fda->entries`, leading to a double-free during error cleanup.
--

--- Patch [1]: [PATCH] tools/lib/api: Check nonfilterable in fdarray__filter() ---
commit f87f1b3c291ca03e96598f96697566a328711de3
Author: Namhyung Kim <namhyung@kernel.org>

    tools/lib/api: Check nonfilterable in fdarray__filter()
    
    IIUC fdarray__filter() should check filterable events only and leave
    non-filterable events to be handled outside.  The recent change to set
    fd to -1 made perf record test stuck forever.  Adding the check the flag
    fixes the problem for me.

> diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
> index ffe8272af59b2..b4570e4981492 100644
> --- a/tools/lib/api/fd/array.c
> +++ b/tools/lib/api/fd/array.c
> @@ -117,6 +117,8 @@ int fdarray__filter(struct fdarray *fda, short revents,
>  	for (fd = 0; fd < fda->nr; ++fd) {
>  		if (!fda->entries[fd].events)
>  			continue;
> +		if (fda->priv[fd].flags & fdarray_flag__nonfilterable)
> +			continue;

[Severity: High]
This is a pre-existing issue, but while looking at this file I noticed a
potential double-free in fdarray__grow() during error handling.

If the realloc for fda->entries succeeds but the realloc for fda->priv
fails, the error path frees the newly allocated entries:

tools/lib/api/fd/array.c:fdarray__grow() {
    ...
	struct pollfd *entries = realloc(fda->entries, size);

	if (entries == NULL)
		return -ENOMEM;

	priv = realloc(fda->priv, psize);
	if (priv == NULL) {
		free(entries);
		return -ENOMEM;
	}
    ...
}

However, fda->entries is neither updated to point to the new entries block
nor cleared to NULL. If realloc moved the allocation to a new block, the
old fda->entries pointer is now freed memory.

When fdarray__exit() is later called to clean up, it executes
free(fda->entries), which would trigger a double-free on that old pointer.

Would it be better to clear fda->entries to NULL here, or avoid modifying
the struct state until both allocations succeed?

[ ... ]

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