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

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

[Namhyung Kim]

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.

Before:
  $ timeout 100 sudo perf test 105
  105: perf record tests                                     : Running (1 active)

  Signal (15) while running tests.
  Terminating tests with the same signal
  Internal test harness failure. Completing any started tests:
  105: perf record tests                                     : FAILED!

  === Test Summary ===
  Passed main tests : 0
  Passed subtests   : 0
  Skipped tests     : 0
  Failed tests      : 1
  List of failed tests:
    105: perf record tests

After:
  $ time sudo perf test 105
  105: perf record tests                                     : Ok

  === Test Summary ===
  Passed main tests : 1
  Passed subtests   : 0
  Skipped tests     : 0
  Failed tests      : 0

  real	0m52.472s
  user	0m0.830s
  sys	0m0.535s

Fixes:f94563fac269 ("perf record: fix poll storm when monitored threads exit")
Cc: Jiawei Sun <abyssmystery@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/lib/api/fd/array.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
index ffe8272af59b2da0..b4570e4981492cd7 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;
 
 		if (fda->entries[fd].revents & revents) {
 			if (entry_destructor)
@@ -132,8 +134,7 @@ int fdarray__filter(struct fdarray *fda, short revents,
 			continue;
 		}
 
-		if (!(fda->priv[fd].flags & fdarray_flag__nonfilterable))
-			++nr;
+		++nr;
 	}
 
 	return nr;
-- 
2.55.0.795.g602f6c329a-goog

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

[sashiko-bot]

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

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

[Namhyung Kim]

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