Re: [PATCH] perf bench: add --write-size option to sched pipe

[Breno Leitao <leitao@debian.org>]

On Fri, May 15, 2026 at 12:05:49PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Integer overflow in write_size cast bypasses pipe capacity validation and causes a deadlock.
> - [High] Missing validation for write_size == 0 causes benchmark hangs or bogus results.
> - [Medium] File descriptor leak on the fcntl error path.

Thanks for the review. Going through them:

> Is there a missing lower-bound check for write_size? If a user passes
> 0, the vfs_write system call will return 0 immediately [...]

Agreed, write_size == 0 is a foot-gun in both modes. I'll add an
explicit rejection in v2.

> Could this cast to (int) cause an unexpected crash? If write_size is
> larger than INT_MAX [...]
> Does this signed cast bypass the pipe capacity validation when
> write_size exceeds INT_MAX?

Right -- the (int) casts on an unsigned int are sloppy. In practice
/proc/sys/fs/pipe-max-size defaults to 1 MiB so the fcntl would refuse
long before INT_MAX, but the comparison logic shouldn't depend on that.

v2 will validate write_size up front (0 < write_size <= INT_MAX) and
drop the casts entirely, so both the BUG_ON checks and the fcntl
return-value comparison become unambiguous.

> Does this return path leak the pipe file descriptors?

Strictly yes, but bench_sched_pipe() returning -1 propagates out to
the perf bench dispatcher and the process exits, so the kernel reaps
the four fds. The surrounding code is consistent with this -- e.g.
the BUG_ON(pipe2(...)) above doesn't unwind either. 

I'd rather keep the style consistent than sprinkle close() calls on one error
path, so I'll leave this as-is unless someone objects.

v2 with the first two fixes coming shortly.