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

[Namhyung Kim <namhyung@kernel.org>]

On Fri, Jul 10, 2026 at 08:01:49PM +0000, sashiko-bot@kernel.org wrote:
> 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.

I'll handle this in a separate patch.

Thanks,
Namhyung

> 
> 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